FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
829 stars 345 forks source link

Problem in implementation of invariant rng-2 #791

Closed ahenket closed 5 years ago

ahenket commented 5 years ago

If you load this xml file in Simplifier validation you'll get two issues related to rng-2, where I would not expect any issue:

Business Rule : Evaluation of FhirPath for constraint 'rng-2' failed: Invocation of 'binary.or' failed: Invocation of 'binary.<=' failed: Function cannot be called with parameters of type 'collection,collection'
MedicationDispense.dosageInstruction[0].rate[0]

Invalid : Instance failed constraint rng-2 "If present, low SHALL have a lower value than high"
MedicationDispense.dosageInstruction[0].rate[0]

This concerns the following fragment which in the Schematron validation for rng-2 validates just fine:

      <rateRange>
         <low>
            <value value="0.2"/>
            <unit value="ml/h"/>
            <system value="http://unitsofmeasure.org"/>
            <code value="ml/h"/>
         </low>
         <high>
            <value value="0.5"/>
            <unit value="ml/h"/>
            <system value="http://unitsofmeasure.org"/>
            <code value="ml/h"/>
         </high>
      </rateRange>

mp9-zib-AdministrationAgreement-MBH-413-13-TA-2.txt

ewoutkramer commented 5 years ago

Duplicate of #447.

Summary: rng-2 is incorrect - it assumes that the < and > operators also work on Quantity - this is however not defined for FhirPath. This will get fixed in R4 in combination with the normative edition of FhirPath - which does support ordering of Quantities.

Note: this is actually pretty hard, because you'd expect 500 grams < 1 kg to work.

The only thing we can do for now in DSTU2/3 is remove the rng-2 rule from the Range datatype. Or hack the FhirPath evaluator to always return true on these comparisons.

ahenket commented 5 years ago

Maybe a slightly more sophisticated hack is conceivable:

My assumption based on what I've seen so far in practice is that 10 g - 1 kg will occur significantly less than 10 g - 1000 g. This hack means that the majority of cases could be validated, which is useful.

ewoutkramer commented 5 years ago

I could do that I think - but you could not expect that to work on all validators (but how many are there right now?)

ahenket commented 5 years ago

I mostly use Touchstone, which is based on an Aegis tweaked validator from Grahame. And then there's Simplifier. I don't know that Grahames validator has the same implementation issue that the .Net validator currently has. But leaving the issue in because other validators have the issue too, is not what you were hinting at? :-)

ewoutkramer commented 5 years ago

Nah ;-)

ewoutkramer commented 5 years ago

We're doing this the right way - I have added Quantity to the FhirPath evaluator - will be integrated in the 1.3 (Sprint 2019.2)

ewoutkramer commented 5 years ago

An update on this situation: since the update of the FhirPath engine is taking more time (read: was reprioritized), we need a quick fix for this. So, for STU2+3 we will just remove this (incorrect) invariant and add it back to STU3 when the FhirPath evaluator is updated.

ewoutkramer commented 5 years ago

Closing this, but see #1024.