Stream: IG creation
Topic: ValueSet Rendering
Lloyd McKenzie (Feb 01 2020 at 03:22):
@Robert McClure has pointed out that the current rendering of value sets is somewhat wonky. The issue is the fragment for the Content Logical Definition. As an example, take a look at this: http://build.fhir.org/ig/HL7/sdc/ValueSet-species.html
Right under the level 3 heading for "Content Logical Definition", we have a level 2 heading that (unnecessarily) repeats the name of the value set. Then we have the description of the value set and the copyright - but that's already covered earlier on in the summary - and has nothing to do with the "content logical definition".
I'd like to strip out that heading, the definition and the copyright declaration from the CLD fragment. Does anyone have any concerns with this?
Grahame Grieve (Feb 01 2020 at 03:36):
I'm ok with that change (though it's not a simple fix because the underlying rendering code has multiple uses)
Rob Hausam (Mar 05 2020 at 14:42):
@Grahame Grieve @Lloyd McKenzie Where are we with this? It doesn't appear that anything has happened with it yet. The default rendering (unless you override the generated text) is rather ugly and problematic, particularly for some of the value sets that we are using now in IPS that are based on the GPS - the formatted copyright information shows up nicely in the table, but the redundant text (including the repeated copyright details) below it is not only unnecessary and distracting but it also doesn't process the markdown - see the allergy reaction example here.
Rob Hausam (Mar 05 2020 at 14:47):
And another issue with the rendering is that if (as in the above example) a compose.include.version is set, that is not displayed anywhere, and the text for "Expansion based on ..." may be incorrect. In this instance the SNOMED version has been set to the 20190731 version of the International Release (as that is what the current GPS is based on), but the text for the expansion says "Expansion based on SNOMED CT International edition 31-Jan 2020".
Lloyd McKenzie (Mar 05 2020 at 17:05):
I guess the first step is a change request. Created here: FHIR#26458
Lloyd McKenzie (Mar 05 2020 at 17:05):
I'd suggest creating a different issue for the release issue as I think that would be a different issue to fix.
Rob Hausam (Mar 05 2020 at 19:42):
@Lloyd McKenzie Should FHIR#26458 be FHIR-I or Vocab? Probably we want both to weigh in.
Rob Hausam (Mar 05 2020 at 20:06):
Here's the tracker for the fixed version issue: FHIR#26462
Grahame Grieve (Mar 05 2020 at 20:21):
the text for "Expansion based on ..." may be incorrect
On what grounds do you think it's incorrect?
Grahame Grieve (Mar 05 2020 at 20:23):
Should FHIR#26458 be FHIR-I or Vocab?
Neither. It belongs to a group we haven't yet created that approves functional changes to the IG publisher
Grahame Grieve (Mar 05 2020 at 20:26):
but this particular one should not a jira issue anyway, since it's actually a bug.
Grahame Grieve (Mar 05 2020 at 20:27):
well, it's a matter of perspective
Rob Hausam (Mar 05 2020 at 20:29):
@Grahame Grieve The reason I think it's incorrect is because (for example) in the value set definition compose.include.version is being set to http://snomed.info/sct/900000000000207008/version/20190731, but when the value set is expanded the message says:
Expansion based on SNOMED CT International edition 31-Jan 2020
So either the message is wrong, or the tx server isn't paying attention to the version specified in the definition (I'll verify which it is).
Grahame Grieve (Mar 05 2020 at 20:30):
I'll verify which it is
yes please
Lloyd McKenzie (Mar 05 2020 at 21:16):
I think FHIR#26458 is purely a rendering issue. We're not changing what information is presented, just ensuring it's all in one place and the heading hierarchy isn't broken. That's not really a content discussion. However, if vocab wants to discuss, they can I guess...
Rob Hausam (Mar 05 2020 at 21:19):
Yes, I think that's correct. I can probably live with that answer (and let FHIR-I do the work). :) Vocab can still take a look and comment if that seems useful.
Rob Hausam (Mar 09 2020 at 22:22):
Do we have any kind of ETA on the updates for FHIR#26458? Assuming that won't be immediate, I think for the IPS publication we will manually create the ValueSet.text for all of our ValueSet resource instances in the IG so that the rendering will be cleaner.
Jose Costa Teixeira (Mar 09 2020 at 22:33):
I could try to have a go at it (it will be useful for my work) but I can't see this in the template.
I think this extra content (which seems indeed strange to be duplicate) comes from {{[type]}}-{{[id]}}-cld.xhtml - where is this created in the template?
@Lloyd McKenzie ?
Lloyd McKenzie (Mar 09 2020 at 22:50):
It's hardcoded in the Java. And as Grahame mentioned, that code is used in multiple places, so it's not a simple change.
Rob Hausam (Mar 09 2020 at 22:58):
Is there a reason that we couldn't branch and execute different code for rendering the value sets?
Lloyd McKenzie (Mar 09 2020 at 23:07):
You can branch to test, but we don't want people using forks of the publisher
Grahame Grieve (Mar 10 2020 at 00:23):
I think those fixes are deployed now
Rob Hausam (Mar 10 2020 at 01:19):
(deleted)
Rob Hausam (Mar 25 2020 at 18:30):
Something seems to have changed and this is again a significant issue. As I mentioned on March 9, in IPS we manually added ValueSet.text which considerably cleaned up the rendering, particularly for the large extensionally defined value sets (e.g. from SNOMED GPS). But for some reason that is no longer working and the generated text from the publisher is being displayed, including the full extensional definitions (which are essentially redundant content with the expansions but also are not truncated to display "only" 1000 concepts!). For the full GPS value set this means that the entire set of 21789 concepts in the definition are being shown (I certainly didn't count them on the page, but it looks like they're probably all there), in addition to the subset of 1000 that is also shown for the expansion. I thought that I would try changing the text.status to "additional", rather than "generated" to see if that would affect the rendering, but it didn't change anything. We do need some kind of a solution now for this, so we can publish IPS with this content included.
Grahame Grieve (Mar 25 2020 at 19:54):
which value set is this?
Rob Hausam (Mar 25 2020 at 21:47):
It's not just one. We have a few GPS value sets (and will have more when we are done), and I think some other larger extensionally defined ones, and it applies to all of them. But the largest one is snomed-intl-gps (it's in the latest CI build).
Rob Hausam (Mar 26 2020 at 22:44):
@Grahame Grieve Have you had a chance to look at this? The change occurred in 1.0.71. The PR that I submitted restores the rendering of ValueSet.text (as it was prior to 1.0.71) and that takes care of our issue. Without that, as it is now, the ValueSet pages can be incredibly large and also incredibly slow to build. With IG Publisher 1.0.73, generating the output for ValueSet/snomed-intl-gps takes about 28 minutes (on my machine), but it only takes 2 seconds using the patched 1.0.73 with the ValueSet.text rendering restored.
Grahame Grieve (Mar 27 2020 at 03:03):
ok I've figured out the context now. There was a reason why this change was made
Rob Hausam (Mar 27 2020 at 03:04):
I figured there was. What is it?
Grahame Grieve (Mar 27 2020 at 03:05):
The problem is that the narrative of the resource is a representation of resource not just the cld. SO people were complaining about the representation of value sets because the cld contained extra duplicate information. The proposed solution was to remove that from the resource, but the context belongs in the resource.
Inb the IG context, the cld is a rendering of the cld, but the resource. And the code you are looking there is the code to generate the cld.
Rob Hausam (Mar 27 2020 at 03:11):
Yes, I am aware of the issue and completely agree with what you are saying. Except that I'm not sure what you mean by "but the context belongs in the resource". But what we have now does not work for the larger extensionally defined value sets. We need something else. Being able to use Valueset.text to explicitly specify the cld text was a compromise that at least allowed a somewhat reasonable rendering and a workable build time, when you needed it (even if technically not correct).
Rob Hausam (Mar 27 2020 at 03:14):
I would be even (much) happier to figure out a better way to do it.
Rob Hausam (Mar 27 2020 at 03:28):
What I think we need is a way to either turn off the automatic generation of the enumerated cld (and possibly replace it with descriptive text). That could be done automatically if the number of concepts in an extensional definition exceeds a particular threshold (maybe 200?), or maybe a "flag" to do that could be manually set (that would presumably be set outside of the ValueSet resource itself - probably in the IG resource entry for the ValueSet resource instance?).
Grahame Grieve (Mar 27 2020 at 03:33):
so I think it needs to be some flag somewhere, yes. I'm trying to think what the most appropriate and simplest option is
Rob Hausam (Mar 27 2020 at 03:40):
Actually, maybe this could be even a lot simpler than anything that I just said. For almost all extensionally defined value sets there is little or no value in listing the concepts in the cld, as they will be immediately repeated in the expansion. So maybe the default should be to never display the contents of compose.include.concept. The only time that it could make sense, maybe, is in the much less common case where compose.inactive = false and the cld and the expansion might be different - maybe the rule should be to only enumerate the compose.include.concept contents in that specific case?
Rob Hausam (Mar 27 2020 at 03:56):
(deleted)
Grahame Grieve (Mar 27 2020 at 10:37):
well, kind of depends whether you think a supplied expansion is reliable. I think... not
Grahame Grieve (Mar 27 2020 at 11:40):
ok so here's what I'll do: I'll look in the narrative and if I see a root level div with an id which is "cld", then I'll treat that as the set rendering for the cld.
<div xmlns="[xhtml]"> <div id="cld">cld rendering goes here</div> </div>
Rob Hausam (Mar 27 2020 at 13:12):
No, I wasn't talking about a supplied expansion - only supplied text. The generated expansion is what we want - just don't also repeat it in the cld rendering.
Rob Hausam (Mar 27 2020 at 13:13):
Your proposal sounds like a simple and good solution, though.
Rob Hausam (Mar 27 2020 at 13:27):
We still will have the issue, though, that DomainResource.text is intended to be the text for the entire resource instance. But what we want is to have the generated text for all of the resource except for the cld, and to be able to supply the text to be rendered only for that. Your proposal, though, probably still is a reasonable compromise to be able to get the result that we need.
Rob Hausam (Mar 27 2020 at 18:43):
Also, I'll circle back again and say that if the narrative generation for the value set cld was set so that it would not enumerate the concepts in the cld for extensionally defined value sets (those using compose.include.concept) where the number of included concepts exceeds a threshold of maybe 200, then I think that would work, without any need to do anything special (or technically incorrect) using ValueSet.text. I'll take a look at the code.
Grahame Grieve (Mar 27 2020 at 18:48):
yes but I won't do that since the expansion doesn't necessarily include all the concepts in the enumeration
Rob Hausam (Mar 27 2020 at 19:36):
Yes, the expansion doesn't necessarily include all of the concepts in the enumeration. But in what circumstances does that occur and when would it be a significant concern? For extensional (enumerated) value set definitions usually the enumeration of the cld and the expansion are going to be the same. The cases where they are not (or might not be) are: (1) ValueSet.compose.inactive = false (or if the particular server default behavior is equivalent to that); (2) $expand activeOnly = true; and (3) the expansion is truncated because it contains too many (i.e. > 1000 concepts). Have I missed anything? For #1, I think we can say that because the cld might be different from the expansion in that case we'll always enumerate it. I think that setting compose.inactive = false is done infrequently enough that I don't have any concern about making that the rule. For #2, passing activeOnly = true to $expand isn't something that we have a way to do or control in the context of IG publishing (as far as I can recall). But if it is possible that activeOnly may be set to true in some cases for IG publishing, then we can treat it the same as #1.
So #3 is where the issue is. For enumerated value sets with > 1000 (or even > 200) concepts, is it practically useful to display all of those concepts in the definition section of the value set page in the IG? Can anyone read that content and make any meaningful use or comparisons from it? My argument is that they can't - and if they can't, and if having that content on the page actually makes reading the page more difficult, they why should we have it there? Also, as I mentioned before, the IG Publisher has terrible performance when generating the pages for value sets that have that much (or considerably more) enumerated content, so there needs to be some means of suppressing that output. Can't the default for those cases be that the enumerated cld contents are not displayed on the page at all (which at least isn't inconsistent, and is what the ValueSet.text workaround does) and that the expectation in those cases is that if you need to have the actual contents of the value set definition you get that from the value set resource instance itself?
Rob Hausam (Mar 27 2020 at 20:04):
@Grahame Grieve Thanks for the commit on looking for the <div id="cld">. That allows us to deal with the current issue. But I think the above is still something to consider.
Roeland Luykx (May 21 2021 at 07:29):
hi
i have a question about valueset rendering in the CLD part. we have in two different implementation guides two different renderings.
one http://build.fhir.org/ig/ehealthsuisse/ch-emed/ValueSet-ActivePharmaceuticalIngredient.html where we have first the table with code/display and then the translation table. in the second example http://build.fhir.org/ig/ehealthsuisse/ch-vacd/ValueSet-ch-vacd-laboratory-serology-vs.html we have directly the translation table.
can this be changed by a flag or definition somewhere in the IG? both pages are builded with the same version.
Roeland Luykx (May 21 2021 at 09:26):
sorry, the problem seems disappeared.. has there something changed on the build server?
Lloyd McKenzie (May 21 2021 at 12:16):
@Grahame Grieve
Grahame Grieve (May 24 2021 at 23:25):
nothing changed in this space in the last month, at least
Elliot Silver (Aug 26 2021 at 01:25):
I have an IG with a ValueSet in it that is rendering oddly.
On the Narrative tab of the ValueSet, the summary shows version 0.1.0, which is the version of my IG. However, my ValueSet resource is version "20160311", which is the value I would have expected to show up here.
Also, the logical definition says "All codes from http://terminology.hl7.org/2.1.0/CodeSystem-v3-AcknowledgementCondition" however there is no ValueSet.compose element (the ValueSet is defined entirely by the expansion), so I don't see how it knows that the valueset is "all codes". Further, it isn't actually all codes -- the current definition of the code system includes one concept that isn't present in my value set.
Grahame Grieve (Aug 26 2021 at 01:35):
have you tried clearing your txCache?
Elliot Silver (Aug 26 2021 at 01:45):
I have now. Still getting the same problems.
Grahame Grieve (Aug 26 2021 at 01:48):
how do I reproduce?
Elliot Silver (Aug 26 2021 at 01:54):
Just pushed to the tx project on https://github.com/ElliotSilver/infoway-tx
Grahame Grieve (Aug 26 2021 at 01:59):
what do I run?
Last updated: Apr 12 2022 at 19:14 UTC
