FHIR Chat · Slicing a non-repeating element · conformance

Stream: conformance

Topic: Slicing a non-repeating element

Pieter Edelman (May 17 2019 at 09:54):

Hi all,

The FHIR documentation states: "One common feature of constraining StructureDefinitions is to take an element that may occur more than once (e.g. in a list), and then split the list into a series of sub-lists [...] this operation is known as "Slicing" a list". This seems to imply that slicing is only allowed for elements with a cardinality > 1. Indeed, the Java validator complains about "Attempt to a slice an element that does not repeat" when slicing an element with cardinality 0..1 or 1..1.

Is this really the case? And if it is, why? There are many use cases where you want to say: "this element may occur just once, but when you do, you may choose from one of these types/coding systems/etc". This can be done quite naturally with slicing and the slicing cardinality rules seem perfectly happy with this.

Marten Smits (May 17 2019 at 10:01):

Hi Pieter,
This topic has been discussed before here: https://chat.fhir.org/#narrow/stream/179166-implementers/topic/Slicing.20non-repeating.20elements.20to.20define.20a.20choice
I don't know what the status of this is though. @Grahame Grieve ??

Pieter Edelman (May 17 2019 at 10:03):

Hi @Marten Smits , thanks for the pointer ... for some reason it didn't show up when I searched the discussions with exactly these search terms.

Alexander Henket (May 17 2019 at 10:45):

Question is: is this still accurate, since it's a discussion from 2017 and Grahames last remark is that he's going to work on it....

Grahame Grieve (May 17 2019 at 22:22):

I think it's all done an implemented

Michel Rutten (May 20 2019 at 11:07):

The .NET API implementation and Forge certainly support this, exactly for the mentioned use case, to define type-specific constraints for a non-repeating choice type element.

Michel Rutten (May 20 2019 at 11:07):

In other words, slicing applies to list elements and choice type elements.

Grahame Grieve (May 20 2019 at 11:07):

so the java stuff used to, right up until you guys convinced me not to

Michel Rutten (May 20 2019 at 11:12):

We did...? Forge STU3 has supported type slicing on choice type elements for a long time. Differential allows for a shorthand notation (w/o slicing introduction), but type slices are always fully expanded in the snapshot.

Grahame Grieve (May 20 2019 at 11:13):

evening decision in Jan meeting last year, leading to this paragraph:

Grahame Grieve (May 20 2019 at 11:14):

http://build.fhir.org/elementdefinition.html#typesx

Michel Rutten (May 22 2019 at 09:05):

That paragraph describes the behavior/interpretation of constraints on choice type elements, however it is silent about slicing. Does this section (2.30.0.4.2) intend to imply that slicing is actually forbidden for choice type elements? Because I interpreted the new R4 behavior as a (compatible) extension to existing behavior in STU3.

Currently, Forge R4 allows users to author type-specific constraints for a choice type element, by toggling the original choice type ("[x]") element into "slicing mode". The application then auto-initializes the slicing discriminator for a type slice to the default value { type = "type", path = "$this" }. The default slicing component is excluded from the generated differential (redundant), but included in the generated snapshot.
The user can then add one or more named slices to specify type-specific constraints. If a named slice is constrained to a single type, then the element is renamed by replacing the "[x]" suffix with the type name, and the sliceName is initialized from the element name. Theoretically, it is even possible to introduce a named slice that applies to multiple types (subset of the original type list); such an element constraint is not renamed, but still recognizable by means of the (custom, author-assigned) slice name.

Allowing slicing on choice types:

• allows the user to further constrain the default slicing discriminator, e.g. by adding additional terms to the discriminator
• Uniquely identifies constraints for specific (sets of) types, because the associated constraints (must) specify a slice name
• Naturally extends existing behavior as defined in STU3
• Does not bloat the differential component for common cases

Grahame Grieve (May 22 2019 at 19:45):

I understood it to be replacement behavior when we discussed it i the evening session and agreed to it in committee. @Lloyd McKenzie @Ewout Kramer @Chris Grenz where there too in the evening session - can't remember who else was.

Lloyd McKenzie (May 22 2019 at 19:51):

I had actually thought there would still be slicing, but have adapted to the fact there wasn't.

Chris Moesel (May 22 2019 at 20:46):

Today I learned something. I am also still slicing value[x] to apply constraints on specific types in the choice. Good to know. I have a slightly related question on this:

Constraints limiting the acceptable list of types must be applied to the original "[x]" element as this is where the list of acceptable types is defined

If you are limiting it to a single allowed type (e.g., limit value[x] to Quantity only) must you keep value[x] on the id/path or can you change it to valueQuantity? In the past, I've changed it; not sure if that's still valid.

Grahame Grieve (May 22 2019 at 21:02):

if the validator sees valueQuantity, it will apply any constraints found there to a Quantity, but if some other type is present, it will assume that this is ok, and the quantity constraints do not apply

Michel Rutten (May 23 2019 at 12:34):

So for R4 choice type constraints, Forge does NOT include slicing discriminator in the differential (unless further constrained by the author). However the .NET snapshot generator for R4 currently expands choice type constraints to include the (implied) slicing component in the snapshot. Is that behavior wrong?

Michel Rutten (May 23 2019 at 12:47):

BTW i do remember the evening session where we discussed R4 type slicing, especially

• allowing multiple type-specific constraints, and
• allowing the (default, implied and therefore redundant) type slicing discriminator to be omitted from the differential.

However I don't recall that we explicitly forbid (custom) discriminator on type slices? Also, I assumed that the snapshot component should still provide a full expansion of the slicing component. Seems to make sense, as this still introduces slicing into the profile and in general, validators then need access to the discriminator.

Michel Rutten (May 23 2019 at 12:49):

Note that §2.30.0.4.2 does not mention anything about the slicing component.

Michel Rutten (May 23 2019 at 12:50):

From §5.1.0.11 Discriminator:

type: Used to match slices based on the type of the item. While it can be used with polymorphic elements such as Observation.value[x], ...

Grahame Grieve (May 24 2019 at 21:39):

Clearly we have different recollections of the discussion that evening. I've asked FHIR-I to take it up on this weeks call

Lloyd McKenzie (May 24 2019 at 23:48):

There isn't scheduled to be a call Monday - due to the US holiday

Grahame Grieve (May 25 2019 at 00:09):

hmm. maybe devdays then

Michel Rutten (May 27 2019 at 09:15):

@Grahame Grieve Issue GF#12259 describes resolution for R4. I interpreted this as allowing for a shorthand notation (w/o explicit slicing), while still allowing the verbose form (with explicit slicing) and custom type slicing discriminators. Also see the first comment:

Note- there is other reasons to use @type - retain the functionality.

This GForge issue does not specifically mention rules for generating the snapshot. The .NET API snapshot generator for FHIR R4 currently always expands the full slicing introduction for all sliced elements, including type slicing and extensions.
I guess this decision mainly affects implementations of FHIR validators, and also code/form generators. Removing verbosity from the snapshot shifts some responsibility to consuming logic, which must be capable to correctly interpret the shorthand notation.

Chris Grenz (May 29 2019 at 20:07):

What's the outstanding question here @Grahame Grieve ? Can you change the discriminator at element[x]? No, it's already sliced. You could re-slice.

Grahame Grieve (Jun 10 2019 at 17:33):

ok. Sonara this evening (devdays)
6:30 packaging and composition
7:15 slicing and choice types

Michel Rutten (Jun 10 2019 at 23:48):

I'll be there

Grahame Grieve (Jun 11 2019 at 10:52):

Outcomes from the discussion:

• we agreed that the specification failed to be clear
• the task disposition is also not clear
• we agreed that this is a serious problem that needs resolution ASAP
• we didn't believe that there's convincing requirements for re-slicing in this case
• there is the CDA problem (forgot to mention this last night) where elements do not have [x] in the name, but have a type choice, so can't be done by renaming?
• the general feeling was the type slicing is more 'regular' with regard to snapshots
• we noted one of Chris's original concerns that the path is ambiguous where you use slicing without renaming the slices
• we ran out of time to resolve but we agree that we need to close it out this week

Alexander Zautke (Jun 11 2019 at 20:38):

There are a few pop up sessions left tomorrow. Do we want to schedule another discussion?

Grahame Grieve (Jun 11 2019 at 20:43):

I think that would be good

Grahame Grieve (Jun 11 2019 at 20:43):

I can do the 11:05 session

Alexander Zautke (Jun 11 2019 at 21:32):

I scheduled the meeting

Alexander Zautke (Jun 11 2019 at 21:33):

Tomorrow 11:05 in Sonora

Grahame Grieve (Jun 12 2019 at 18:45):

Choices

1. no slicing on snapshots - renaming only
2. renaming in differentials, slicing on snapshots
3. renaming or slicing in differentials, slicing in snapshots

Grahame Grieve (Jun 12 2019 at 18:45):

decision - #3

Grahame Grieve (Jun 12 2019 at 18:45):

@Chris Grenz @Ewout Kramer is counting the minutes....

Chris Grenz (Jun 13 2019 at 09:58):

Can you illustrate via an example?

Grahame Grieve (Jun 13 2019 at 11:52):

@Ewout Kramer 3:13 :grinning:

Grahame Grieve (Jun 13 2019 at 11:52):

@Chris Grenz the renaming syntax documented here:

Grahame Grieve (Jun 13 2019 at 11:53):

http://hl7.org/fhir/elementdefinition.html#typesx

Grahame Grieve (Jun 13 2019 at 11:54):

will be treated as one optional way to do the differential. Or you can do classic @type based slicing. Either way, what goes in the snapshot will be classic type based slicing.

Richard Townley-O'Neill (Jun 17 2019 at 05:05):

@Grahame Grieve Do you expect the igpublisher to currently produce classic type based slicing in the snapshot?

Grahame Grieve (Jun 17 2019 at 05:06):

right now I don't know what it's going to do

Richard Townley-O'Neill (Jun 17 2019 at 05:07):

HL7 AU has ingredient in differential with slicing and in snaphot without
http://build.fhir.org/ig/hl7au/au-fhir-base/StructureDefinition-au-medication.html
http://build.fhir.org/ig/hl7au/au-fhir-base/StructureDefinition-au-medication.html#tabs-snap

Grahame Grieve (Jun 17 2019 at 05:23):

well, right now, we're just broken on here due to confusion between the various tool smiths. I think we sorted it out last week, but it will be some time before we get all aligned

Richard Townley-O'Neill (Jun 17 2019 at 05:24):

Thanks. I wanted to check my understanding. Things will get better. :grinning:

Grahame Grieve (Jun 23 2019 at 21:08):

ok. so. here's my test cases

Grahame Grieve (Jun 23 2019 at 21:09):

choice-slicing-tests.xml

Grahame Grieve (Jun 23 2019 at 21:11):

@Lloyd McKenzie @Michel Rutten @Ewout Kramer please check that these differentials represent the right set of test cases. For the latter tests, the snapshot output should the same (43/44 & 45/46)

Lloyd McKenzie (Jun 23 2019 at 21:43):

I get a bad document error when I click the link

Grahame Grieve (Jun 23 2019 at 21:49):

open it in a text editor - it's not valid xml

Grahame Grieve (Jun 23 2019 at 21:49):

there's multiple root elements

Lloyd McKenzie (Jun 23 2019 at 23:23):

I mean just clicking the link in zulip. Tried opening in spy and link didn't resolve

Lloyd McKenzie (Jun 24 2019 at 00:44):

So there are no magic slice names? I think there should be. Otherwise when constraining downstream slices by name, the behavior will be different whether an element is named "x.value[x]" (where the slice name is manually assigned) vs. "x.valueCodeableConcept" (were the slice name will be auto-generated)

Lloyd McKenzie (Jun 24 2019 at 00:44):

(I was able to get the file by just downloading it.)

Lloyd McKenzie (Jun 24 2019 at 00:44):

Other than that, I saw no issues.

Grahame Grieve (Jun 24 2019 at 10:39):

I don't know what you mean by magic slice names. It's not ringing a bell for me

Lloyd McKenzie (Jun 24 2019 at 10:42):

You had slicenames of "codeable" and "quantity". I thought we'd agreed to use "CodeableConcept" and "Quantity". I.e. The sliceName would exactly match the type name if we were doing type-based slicing on a polymorphic element.

Grahame Grieve (Jun 24 2019 at 10:53):

I don't recall any such agreement? is this written down anywhere?

Lloyd McKenzie (Jun 24 2019 at 10:57):

Not written down. I just recall it as a verbal discussion. Key question is as follows:
- If you use the shortcut, then behind the scenes the code's going to inject a slice name. That slicename will be algorithmicly determined. If we allow those who don't use the shortcut to manually define an arbitrary slice name that's different from the algorithmicly determined slice name, is that inconsistency going to cause issues?

My concern is that someone who switches from shortcut to full expression or vice-versa could break downstream models (that are constraining slices). I don't think that should happen.

Grahame Grieve (Jun 24 2019 at 11:13):

I don't recall the discussion. I agree that changing the slicing could impact downstream slicing, but that's true of any change to the slicing...

Lloyd McKenzie (Jun 24 2019 at 16:08):

Changing shortcut to/from non- shortcut is a representation change, not a content change. Furthermore, it could be tooling-driven. One tool might save using the shortcut. Another might not. You don't want switching tools in the parent model to break child models.

Lloyd McKenzie (Jun 24 2019 at 16:09):

Do you see a problem with enforcing standard slice names for this case?

Grahame Grieve (Jul 05 2019 at 00:41):

well, that depends. I'm ok with generating standard names, but should the snapshot generator reject other names?

Lloyd McKenzie (Jul 05 2019 at 01:19):

I think it should. There's no good reason to have inconsistency and having it creates pain

Grahame Grieve (Jul 05 2019 at 02:33):

we don't do that anywhere else. that would imply Forge should force consistent names too, at least here. @Michel Rutten

Lloyd McKenzie (Jul 05 2019 at 13:59):

We don't have shortcuts that allow generation of names anywhere else...

Grahame Grieve (Jul 06 2019 at 22:24):

ok. updated the tests at https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/main/resources/snapshot-generation-tests.xml. I still have to write the assertions for Tests 45 & 46, but @Lloyd McKenzie please check the assertions for 43/44 first

Grahame Grieve (Jul 06 2019 at 22:24):

40 - 42 pass. 43/44 fail until I update the snapshot generation.

Grahame Grieve (Jul 06 2019 at 22:25):

@Michel Rutten please check these as well

Ewout Kramer (Jul 08 2019 at 08:30):

@Michel Rutten has been on holidays for the past two weeks, so I am sure he'll be in the mood and eager to pick this up this week ;-)

Grahame Grieve (Jul 15 2019 at 12:27):

@Lloyd McKenzie getting to this... it means that tests 5 and 6 are also broken too.

Grahame Grieve (Jul 15 2019 at 12:28):

Will check in a fix for what I think they should be

Grahame Grieve (Jul 15 2019 at 12:28):

in fact, I think that T6 doesn't make any sense any more:

Grahame Grieve (Jul 15 2019 at 12:28):

    <StructureDefinition>
      <id value="t6"/>
      <url value="urn:uuid:977f7fee-448e-4090-8c3a-441b3b3cca7a"/>
      <name value="t6"/>
      <status value="draft"/>
      <description value="fixture for #6: type narrowing #2 - this renames  the type element"/>
      <kind value="resource"/>
      <abstract value="false"/>
      <type value="Patient"/>
      <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Patient"/>
      <derivation value="constraint"/>
      <differential>
        <element>
          <path value="Patient.deceasedDateTime"/>
          <type>
            <code value="dateTime"/>
          </type>
        </element>
      </differential>
    </StructureDefinition>

Grahame Grieve (Jul 15 2019 at 12:29):

that's a null statement, I think

Grahame Grieve (Jul 15 2019 at 12:29):

@Michel Rutten have you looked at this thread?

Grahame Grieve (Jul 15 2019 at 13:46):

the last question leads me to a related question:

Grahame Grieve (Jul 15 2019 at 13:46):

Do these 2 xml fragments have the same meaning?

Grahame Grieve (Jul 15 2019 at 13:46):

    <StructureDefinition>
      <id value="t44"/>
      <url value="urn:uuid:ecb6f563-2957-4da8-832e-cb6d94329a93"/>
      <name value="t44"/>
      <status value="draft"/>
      <description value="fixture for #44: Choice Types: constrain list of choices to 1 and constrain the type (shortcut)"/>
      <kind value="resource"/>
      <abstract value="false"/>
      <type value="Observation"/>
      <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Observation"/>
      <derivation value="constraint"/>
      <differential>
        <element>
          <path value="Observation"/>
        </element>
        <element>
          <path value="Observation.value[x]"/>
          <short value="some text"/>
          <type>
            <code value="Quantity"/>
          </type>
        </element>
        <element>
          <path value="Observation.valueQuantity"/>
          <short value="some text for quantity"/>
        </element>
        <element>
          <path value="Observation.valueQuantity.value"/>
          <min value="1"/>
        </element>
      </differential>
    </StructureDefinition>

Grahame Grieve (Jul 15 2019 at 13:47):

    <StructureDefinition>
      <id value="t44"/>
      <url value="urn:uuid:ecb6f563-2957-4da8-832e-cb6d94329a93"/>
      <name value="t44"/>
      <status value="draft"/>
      <description value="fixture for #44: Choice Types: constrain list of choices to 1 and constrain the type (shortcut)"/>
      <kind value="resource"/>
      <abstract value="false"/>
      <type value="Observation"/>
      <baseDefinition value="http://hl7.org/fhir/StructureDefinition/Observation"/>
      <derivation value="constraint"/>
      <differential>
        <element>
          <path value="Observation"/>
        </element>
        <element>
          <path value="Observation.valueQuantity"/>
          <short value="some text for quantity"/>
        </element>
        <element>
          <path value="Observation.valueQuantity.value"/>
          <min value="1"/>
        </element>
      </differential>
    </StructureDefinition>

Lloyd McKenzie (Jul 15 2019 at 14:42):

No. The second one constraints the valueQuantity slice. It doesn't prevent the other types from being used. (That was a very clear outcome of our meeting two WGMs ago.)

Grahame Grieve (Jul 16 2019 at 13:01):

so... this invalidates every generated Extension we have ever generated. I have a problem with that

Grahame Grieve (Jul 16 2019 at 13:03):

in fact, it's not a backwards compatible change. Which is yet another problem.

Grahame Grieve (Jul 16 2019 at 13:04):

we need to interpret the second the same as the first....

Lloyd McKenzie (Jul 16 2019 at 14:09):

You agreed to it when we had the discussion long ago. And I thought you'd changed the code to align well before we published R4? Talking about valueQuantity is just talking about a specific slice. Constraining what types are allowed needs to be done on value[x].

Grahame Grieve (Jul 16 2019 at 21:11):

it's not clear to me what I may have thought I was agreeing with, or not, nor is it clear what other people may have thought I was agreeing too, but this is clear:

http://hl7.org/fhir/extension-capabilities.xml.html

Every extension we've ever published has a differential like this. And it's a pattern that exists in other profiles as well. See, for example:

https://simplifier.net/NictizSTU3-Zib2017/nl-core-relatedperson-role/~xml

I don't think that we can change this now. @Michel Rutten

Lloyd McKenzie (Jul 16 2019 at 21:17):

@Chris Grenz

Grahame Grieve (Jul 17 2019 at 11:06):

ok I have committed a set of tests and code based on the idea that we aren't breaking backwards compatibility on this

Grahame Grieve (Jul 17 2019 at 11:11):

https://github.com/hapifhir/org.hl7.fhir.core/tree/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation

Chris Grenz (Jul 17 2019 at 20:04):

@Grahame Grieve the language here: http://hl7.org/fhir/elementdefinition.html#typesx specifically exists to make this clear. Note:

Constraints limiting the acceptable list of types must be applied to the original "[x]" element as this is where the list of acceptable types is defined

Chris Grenz (Jul 17 2019 at 20:05):

So path: Observation.valueQuantity is referring to id: Observation.value[x]:valueQuantity and is making no statement about the other value types.

Chris Grenz (Jul 17 2019 at 20:06):

@Michel Rutten has also affirmed this interpretation in his blog post here: https://blog.fire.ly/2019/02/06/fhir-r4-profiling-and-forge/

Chris Grenz (Jul 17 2019 at 20:14):

Even more clear in the R4 spec, the next bullet:

The inclusion of a type specific path (such as "Patient.deceasedBoolean") SHALL NOT be interpreted as constraining allowed types, but instead, it constrains the use of a particular type

Chris Grenz (Jul 17 2019 at 20:16):

This profile: http://hl7.org/fhir/extension-capabilities.xml.html has not been converted properly from STU3 and isn't generating conformant element ids.

Lloyd McKenzie (Jul 17 2019 at 20:33):

That's what I recall. And I thought Grahame had gone in and fixed the code so that the snapshot generator worked this way...

Chris Grenz (Jul 17 2019 at 20:42):

I think we need to re-gen all the SDs in the R4 technical correction, a la GF#22681

Grahame Grieve (Jul 17 2019 at 20:43):

The R4 technical correction is getting out of hand

Grahame Grieve (Jul 17 2019 at 20:43):

but I now have a problem: I cannot manage this process any more.

Grahame Grieve (Jul 17 2019 at 20:44):

the R4 technical correction was slated for once the ballot opened, so that it wasn't competing for resources etc with ballot opening. Since that tends to be all en-compassing for us

Grahame Grieve (Jul 17 2019 at 20:44):

but now, getting the ballot out depends on the R4 technical corrections

Grahame Grieve (Jul 17 2019 at 20:44):

and already, doing the technical corrections (R2-R4) was a reset of the entire tooling base that I expected to take a week....

Chris Grenz (Jul 18 2019 at 08:37):

What support can be offered? What would be helpful?

Michel Rutten (Jul 18 2019 at 13:31):

Hi @Grahame Grieve, apologies for radio silence. As Ewout has mentioned, I took a vacation after DevDays. Now ready to start working on this.
Indeed, the impact on extensions also dawned on me. The simple/compact notation used in DSTU3 no longer limits the list of allowed types in R4. Most R4 extensions I've seen still use the STU3 compact notation, which is interpreted differently in R4, and usually not what the author intends. Instead, all (...) R4 extensions should now always specify both a value[x] ElementDef to limit types, and usually another named slice e.g. valueBoolean to specify further constrains on the allowed type - in the differential! So this is not a matter of simply re-generating the snapshot.

Michel Rutten (Jul 18 2019 at 13:38):

Concerning slice names, we've also received some questions from Forge customers about this.
Forge auto-generates slice names for named type slices, according to the default pattern, e.g. valueBoolean. Currently, the user cannot override/customize these auto-generated slice names. If you open a (manually edited) profile with custom type slice names, then Forge will automatically revert to the default slice names.
Users have been asking us if Forge could allow custom type slice names in R4. For example, one user mentioned a use case where they introduce multiple valueQuantity slices, representing constraints on different units. Of course, this requires assigning unique and therefore custom slice names - which is currently not supported by Forge.
Question: does FHIR allow this? If not, is there an alternative approach?

Lloyd McKenzie (Jul 18 2019 at 15:10):

This would be a case of re-slicing (slicing an existing slice). Wouldn't the id convention distinguish?

Chris Grenz (Jul 18 2019 at 15:19):

Agree - id:"Observation.value[x]:valueQuantity/valueInCm"

Chris Grenz (Jul 18 2019 at 15:21):

The slicing definition at value[x] is set by the standard to type. id:"Observation.value[x]:valueInCm" would be illegal.

Chris Grenz (Jul 18 2019 at 15:22):

As for the STU3 Extension SDs - they need to be converted to R4 format like all resources.

Michel Rutten (Jul 19 2019 at 09:12):

@Chris Grenz if I understand correctly, in your reslicing example, the full sliceName is valueQuantity/valueInCm, where the first segment is defined/prescribed by FHIR standard, and the last segment is custom and author-defined? So to support this scenario, Forge could for example auto-generate the first segment and require authors to specify the remainder?

Chris Grenz (Jul 19 2019 at 10:02):

Yes, as a reslice - the slice of value[x] is valueQuantity and mandated by the standard. The (re)slice of valueQuantity is user defined and user named.

Michel Rutten (Jul 19 2019 at 11:02):

Makes sense, thank you for the feedback!

Grahame Grieve (Jul 19 2019 at 13:19):

What support can be offered? What would be helpful?

@Chris Grenz thanks. Right now the most useful thing someone could do is sort out the gForge tasks for the technical corrections - do we know what they are, are they all approved by FHIR-I? Is there anything that's been reported that's not a task? DO the tool smiths have any last call issues?

Grahame Grieve (Jul 19 2019 at 13:20):

the other useful thing to do would be check the test cases

Michel Rutten (Jul 19 2019 at 13:21):

FYI I'm planning to implement & review the test cases next week.

Grahame Grieve (Jul 19 2019 at 13:21):

https://github.com/hapifhir/org.hl7.fhir.core/tree/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation

Grahame Grieve (Jul 19 2019 at 13:21):

great.

Grahame Grieve (Jul 19 2019 at 13:22):

They're not yet done - currently, I am interpreting the shortcut syntax the old way. I will update them to the new way over the next 2 days

Grahame Grieve (Jul 19 2019 at 13:22):

(they're good other than that)

Grahame Grieve (Jul 19 2019 at 21:59):

fixed. I think they're right now. In particular, T6, T43 and T44 deserve close attention, along with their last delta

Chris Grenz (Jul 20 2019 at 11:34):

For T6, should the snapshot include all type slices once one of them is included? A minor point...more about consistency than correctness...

Chris Grenz (Jul 20 2019 at 11:37):

T43's input should be rejected -> value[x]'s slice for Quantity must be named valueQuantity. And value isn't a child of value[x]. Only Element children can be children of [x] elements.

Chris Grenz (Jul 20 2019 at 11:38):

T44's differential is generating incorrect ids -> <element id="Observation.valueQuantity"> should be <element id="Observation.value[x]:valueQuantity">

Chris Grenz (Jul 20 2019 at 11:40):

And the snapshot goes off the rails @ line 951 with the introduction of the "Quantity" slice to value[x]

Chris Grenz (Jul 20 2019 at 11:41):

I think T44 line 1043 should be <path value="Observation.valueQuantity.value"/> instead of current <path value="Observation.value[x].value"/>

Chris Grenz (Jul 20 2019 at 11:43):

@Michel Rutten is probably best suited to generate a snapshot and PR for these? I don't have any tooling at the moment apart from his...

Grahame Grieve (Jul 21 2019 at 07:36):

T6 - no, I don't think it should.

Grahame Grieve (Jul 21 2019 at 07:42):

T43:

• don't agree about the slice name, see above. (@Lloyd McKenzie). At least I don't think that's what we agreed, but I don't care
• value is a child of value[x]. That we also discussed specifically

Grahame Grieve (Jul 21 2019 at 07:45):

I don't see where the snapshot goes off the rails

Grahame Grieve (Jul 21 2019 at 07:45):

T44 I don't agree either

Grahame Grieve (Jul 21 2019 at 08:49):

ok I pushed corrected differentials to https://github.com/FHIR/packages/tree/master/hl7.fhir.rX/hl7.fhir.r4.examples/package and https://github.com/FHIR/packages/tree/master/hl7.fhir.rX/hl7.fhir.r4.core/package

Grahame Grieve (Jul 21 2019 at 08:50):

note: the snapshots are not correct at this time. I have not regenerated them (problems). So just review the differentials...

Grahame Grieve (Jul 21 2019 at 20:47):

ok all sorted. I think they are correct

Lloyd McKenzie (Jul 21 2019 at 22:00):

My recollection was that the slice names for the short-cut was the 'type', not the element name. I don't really care either, but I do have a slight bias towards shorter - which the type name would be. @Chris Grenz do you have a reason for wanting the element name (beyond the fact that you probably implemented things that way? :))

Chris Grenz (Jul 22 2019 at 06:23):

http://hl7.org/fhir/elementdefinition.html#id defines the id for a type slice and uses the full path name.

Grahame Grieve (Jul 22 2019 at 08:47):

But sure how that’s relevant to the question at hand?

Grahame Grieve (Jul 22 2019 at 08:47):

Grr. Not sure...

Lloyd McKenzie (Jul 22 2019 at 13:57):

@Chris Grenz The question is whether the sliceName should be "Quantity" or "valueQuantity" (not what the id should be). Is there a reason why you think we should use "valueQuantity" rather than "Quantity"?

Michel Rutten (Jul 22 2019 at 14:17):

I seem to remember that we decided to standardize slice names of type slices to e.g. valueBoolean - derived from element name by replacing [x] with the name of associated single type. The current implementations of the .NET FHIR API & Forge (STU3 & R4) follow this pattern.
However section 2.30.0.3 (http://hl7.org/fhir/elementdefinition.html#id) of the R4 spec states:

For type choice elements, the id reflects the type slice. e.g. For path = Patient.deceasedBoolean, the id is Patient.deceased[x]:deceasedBoolean

This line suggests, but does not actually require pre-defined slice names for type slices.
This would imply that Patient.deceased[x]:boolean and Patient.deceased[x]:foo are also valid.

Chris Grenz (Jul 22 2019 at 14:31):

The illustration in the id section indicates it should be valueQuantity. No strong opinion, but think that guidance should tip the scales.

Grahame Grieve (Jul 22 2019 at 20:31):

well, ok. updated the tests. Check them again

Chris Grenz (Jul 25 2019 at 10:44):

@Grahame Grieve Looking at the commit (https://github.com/hapifhir/org.hl7.fhir.core/commit/4274392020cbac6b9e5cf7cfafc3be1e656fda2f#diff-64f5d43722b5cabb662451ca19679f13) it looks like you only changed the slice names. Those look fine.

For Observation.value[x].value and generally for children of [x] type choice elements, how do you know which element is being constrained? If I create a constraint on Extension.value[x].system, where will you apply it? Coding.system or Quantity.system? Both? Element's children are the only ones unambiguous. I certainly wasn't part of any agreement on this (not that I have to be!).

Will re-check others on plane...

Chris Grenz (Jul 25 2019 at 12:28):

On path - should the path be reflective of the instance structure? For id:"Observation.value[x]:valueQuantity", I assume either path:"Observation.value[x]" or path:"Observation.valueQuantity" would be ok?

This becomes more important with children as in my last comment. value[x].value is ambiguous. valueQuantity.value is not.

Chris Grenz (Jul 25 2019 at 12:30):

https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t44-expected.xml#L1043

Chris Grenz (Jul 25 2019 at 12:32):

At very least, in T44's input (https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t44-input.xml#L30), the element id should be required...although in this case I guess it happens to be unambiguous due to the type limits on value[x].

Chris Grenz (Jul 25 2019 at 12:34):

Appears you're accepting any slice name: https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t43-input.xml#L33

And then converting it to the valueUnit name in the snapshot: https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t43-expected.xml#L958

While leaving the original in the differential: https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t43-expected.xml#L2144

Is this intended?

Grahame Grieve (Jul 25 2019 at 12:41):

yes, the path is now ambiguous; you have to look back up the path parents to see what the type is.

Grahame Grieve (Jul 25 2019 at 12:41):

we specifically discussed this in Seattle while coming to agreement

Chris Grenz (Jul 25 2019 at 12:41):

So what happens with Extension.value[x].system?

Chris Grenz (Jul 25 2019 at 12:42):

assuming I don't limit the types.

Grahame Grieve (Jul 25 2019 at 12:42):

I do not require the element ids on the differential

Grahame Grieve (Jul 25 2019 at 12:42):

and I am testing that. though it is not formally valid

Chris Grenz (Jul 25 2019 at 12:43):

So path:"Extension.value[x].system" should be rejected?

Grahame Grieve (Jul 25 2019 at 12:43):

you can't say anything about Extension.value[x].system unless you limit the types. there's a another test about that somewhere - you can only constrain things on all elements if you don't constrain the type

Chris Grenz (Jul 25 2019 at 12:44):

Meaning you have to check all the remaining types and see if there's any ambiguity?

Chris Grenz (Jul 25 2019 at 12:44):

That's pretty brittle I think...

Grahame Grieve (Jul 25 2019 at 12:44):

and yes, I rewrite the sliceName in the snapshot without fixing it in the differential. I'm open to discussion on the point. I can raise an exception, or fix it (though fixing it is super hard for me). I elected not to. discussion welcome.

Grahame Grieve (Jul 25 2019 at 12:45):

no that situation doesn't arise. Either you have a single type, or you can only talk about element properties

Chris Grenz (Jul 25 2019 at 12:45):

I think it's pretty confusing if the snapshot generator creates identifier inconsistencies between snapshot and differential.

Grahame Grieve (Jul 25 2019 at 12:46):

well, let's see what other people think.

Chris Grenz (Jul 25 2019 at 12:49):

no that situation doesn't arise. Either you have a single type, or you can only talk about element properties

Where can I find this rule? Also, if I limit to 2 types, say Quantity and Coding, and then constrain id:"Extension.value[x].valueQuantity.system" and id:"Extension.value[x].valueCoding.system", both will have path:"Extension.value[x].system". And you don't require ids in the differential...

Chris Grenz (Jul 25 2019 at 12:50):

It seems to me that either ids need to be required, or paths need to be type specific.

Grahame Grieve (Jul 25 2019 at 12:51):

they'll have to be sliced, either fomally, or informally. The rule is... somewhere. probably only in Zulip for now - I think we discussed this about 3 months ago

Grahame Grieve (Jul 25 2019 at 12:51):

if you're talking about the differential, ids are derivative. Either you fomally specify the slices, or you use the short cut. Either way, there is no ambiguity.

Lloyd McKenzie (Jul 25 2019 at 12:53):

My leaning is to raise an error if the differential slice name is wrong. Otherwise you end up with non-matching ids in the snapshot and differential - which should cause an error.

Chris Grenz (Jul 25 2019 at 12:55):

We agree this is an error - it's ambiguous.

{
  "path":"Extension.value[x]",
  "type": [
    { "code":"Quantity" },
    { "code":"Coding" }
  ]
},
// slices valueQuantity and valueCoding implicitly exist already. Don't need to declare.
{
  "path":"Extension.value[x].system",
  "min":1
}

Chris Grenz (Jul 25 2019 at 12:57):

I think you're saying I need to do this:

{
  "path":"Extension.value[x]",
  "type": [
    { "code":"Quantity" },
    { "code":"Coding" }
  ]
},
// slices valueQuantity and valueCoding implicitly exist already. Don't need to declare.
{
  "path":"Extension.value[x]",
  "sliceName":"valueQuantity"
},
{
  "path":"Extension.value[x].system",
  "min":1
}

and then infer the last constraint based on the ordering of the elements.

Chris Grenz (Jul 25 2019 at 12:59):

Which is one of the explicit reasons for the well-formed ids - that you don't have to try and capture meaning from the order of elements in the differential.

Chris Grenz (Jul 25 2019 at 13:01):

Aside...on T6, the snapshot declares the slicing closed (good): https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation/t6-expected.xml#L566

But then fails to include all the slices. I guess we do need to include them all.

Grahame Grieve (Jul 25 2019 at 13:03):

I'm not understanding the point about T6

Grahame Grieve (Jul 25 2019 at 13:03):

I don't mind if other tooling requires properly formed ids. It is required. Just that mine doesn't.

Grahame Grieve (Jul 25 2019 at 13:03):

but the ids by themselves are not enough

Chris Grenz (Jul 25 2019 at 13:03):

Looking again... sdf-14 requires ids in both snapshot and differential.

Chris Grenz (Jul 25 2019 at 13:04):

What scenario/id would be ambiguous?

Grahame Grieve (Jul 25 2019 at 13:04):

ambiguous?

Chris Grenz (Jul 25 2019 at 13:04):

Why aren't ids enough?

Chris Grenz (Jul 25 2019 at 13:05):

With ids, you don't need to infer anything from element ordering.

Grahame Grieve (Jul 25 2019 at 13:05):

you can't imply the existence of slices in the id. You actually have to have the slices present. I didn't say anything about ambiguous

Chris Grenz (Jul 25 2019 at 13:07):

Based on my understanding of our discussions, a differential with a single element, id:"Extension.value[x]:valueQuantity.value" is legal.

Chris Grenz (Jul 25 2019 at 13:07):

Type slices implicitly exist.

Grahame Grieve (Jul 25 2019 at 13:08):

I disagree. I've seen nothing to say that this is the case. It's driven by the path, not the id

Chris Grenz (Jul 25 2019 at 13:08):

@Michel Rutten @Ewout Kramer @Lloyd McKenzie

Grahame Grieve (Jul 25 2019 at 13:09):

the path Extension.valueQuantity.value establishes the implicit type slice. We never said that you could establish it by id

Chris Grenz (Jul 25 2019 at 13:12):

value[x] establishes the implicit type slices

Chris Grenz (Jul 25 2019 at 13:16):

id is just referring to what's already there.

Chris Grenz (Jul 25 2019 at 13:16):

For T6 - a "closed" slicing needs to enumerate all the possible slices, no?

Lloyd McKenzie (Jul 25 2019 at 13:19):

Agree with Grahame that the paths drive, not the ids. You'd have to have an element with "Extension.valueQuantity" or "Extension.value[x]:valueQuantity" because neither of those is explicitly declared in the snapshot of what you're inheriting from - and every element must be explicitly declared either in the parent snapshot or the child differential.

Grahame Grieve (Jul 25 2019 at 13:20):

I am not sure about T6. I don't want to have to enumerate all the slices. The snapshots will be massive

Chris Grenz (Jul 25 2019 at 13:23):

So if I have a single element like this, it's ok?

{
  "id":"Extension.value[x]:valueQuantity.value",
  "path":"Extension.valueQuantity.value",
  "min":1
}

I'm fine with that.

Grahame Grieve (Jul 25 2019 at 13:25):

yes

Chris Grenz (Jul 25 2019 at 13:26):

To be clear: that's the only element in the differential.

Chris Grenz (Jul 25 2019 at 13:44):

Of course, sdf-14 makes this distinction moot since a well-formed id is required on all elements, snapshot and differential.

Chris Grenz (Jul 25 2019 at 13:50):

For T6 - a closed slicing (to me) means that all valid slices have been enumerated, no more are allowed. If you then only enumerate one, type, even though the value[x] type list allows more, the slicing validation won't (only one slice, instance isn't in that slice, error).

Chris Grenz (Jul 25 2019 at 13:51):

Not sure in practice that means that much bloat in snapshot - only would be sizeable when the type choice list is long (e.g. Extension.value[x]), the allowed types aren't constrained, and one type has a constraint. Seems like a rare intersection...

Grahame Grieve (Jul 25 2019 at 20:41):

if you think that the slices should be enumerated if the type list is restricted, then you'll also think that they should be enumerated when the type list is not restricted

Grahame Grieve (Jul 25 2019 at 21:41):

ok I updated t29 and t43 - and added clones. The generation now blows up if the slice names are wrong in the differential. I actually changed t29, so that difference needs checking please

Ewout Kramer (Jul 29 2019 at 09:57):

I disagree. I've seen nothing to say that this is the case. It's driven by the path, not the id

My validator totally disregards the id - it's driven by the path to recognize slices. Then again, I full rely on the snapshot as generated by @Michel Rutten - I don't know how much he used the id to generate the snapshot....

Michel Rutten (Jul 29 2019 at 10:43):

The .NET SnapshotGenerator generates Element Ids, but does not use/depend on them.

Michel Rutten (Jul 29 2019 at 10:44):

i.e. elements are identified strictly by list order, Path & SliceName.

Grahame Grieve (Jul 29 2019 at 10:49):

Same as java

Michel Rutten (Aug 19 2019 at 16:27):

@Grahame Grieve FYI I've managed to pass all your SnapGen unit tests - with some discrepancies/corrections. Tomorrow I will create and submit a report of all my findings.

Grahame Grieve (Aug 19 2019 at 18:41):

ok thanks

Michel Rutten (Aug 20 2019 at 14:49):

Hi @Grahame Grieve, I took all the SnapshotGenerator unit tests from HAPIFHIR github repo:
https://github.com/hapifhir/org.hl7.fhir.core/tree/master/org.hl7.fhir.r5/src/test/resources/snapshot-generation
and re-implemented and executed them on the .NET FHIR library (R4 development branch):
https://github.com/FirelyTeam/fhir-net-api/tree/develop-r4
You can find the results here:
https://gist.github.com/wmrutten/acc5e24601eb74df582ff39b21f41efc

Michel Rutten (Aug 20 2019 at 14:52):

Next, I am going to look into the type slicing change as we discussed during DevDays US in Redmond; specifically to always emit a type slicing entry to the snapshot component, even when omitted from the differential. May have to update our SnapGen logic.

Last updated: Apr 12 2022 at 19:14 UTC