Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attributes on union output not accessible, not yielding expected calculation results #191

Closed
nmorasb opened this issue Jan 25, 2023 · 8 comments
Labels
translator Issues that may involve work with the cql-to-elm translator

Comments

@nmorasb
Copy link

nmorasb commented Jan 25, 2023

It seems that the output of a union inside a library is not allowing access to properties of the union types.
We have a define in a library performing a union:
image

Usage showing attributes are not hit:
image

Usage in the denominator exclusion:
image

It seems like the attributes performed and authoredOn of ComfortMeasure may not be accessible and are both returning null.

The measure bundle and patient bundle are attached for reference.
union-output-issue.zip

@JSRankins

@srankins
Copy link

srankins commented Jan 25, 2023

For whoever looks at this, just to clarify, I think the issue occurs whenever the union results in a choice type. Note: a similar issue occurred with cql-engine. Refer to CQLIT-329.

@mgramigna
Copy link
Contributor

mgramigna commented Jan 26, 2023

@nmorasb @srankins

Thanks! I was able to replicate the issue in fqm-execution. We will try working with a simpler test case I created that also demonstrates this issue:

library UnionTest version '1.0.000'
 
using QICore version '4.1.1'

include FHIRHelpers version '4.1.000' called FHIRHelpers
include QICoreCommon version '1.2.000' called QICoreCommon

parameter "Measurement Period" Interval<DateTime>

context Patient

define "Union Issue":
  ([Procedure] union [ServiceRequest]) ComfortMeasure
    return {
      cmPerformed: ComfortMeasure.performed,
      cmAuthoredOn: ComfortMeasure.authoredOn,
      coalesced: Coalesce(start of QICoreCommon."ToInterval"(ComfortMeasure.performed), ComfortMeasure.authoredOn)
    }

Currently, all of the values in this returned object are null, which shouldn't be the case given the patient bundle that you've provided.

I logged a ticket in our backlog. Will let you know what the path forward is (be it here, cql-execution, or translator).

@mgramigna
Copy link
Contributor

mgramigna commented Jan 26, 2023

Also likely relevant is these errors that are logger during execution (which I think actually come from cql-exec-fhir). This usually indicates an issue with the ELM

image

@mgramigna
Copy link
Contributor

@nmorasb @srankins last question. Which version of the translator are you using? Just to make sure we have the latest if we try to replicate this with other CQL.

@nmorasb
Copy link
Author

nmorasb commented Jan 26, 2023

Interesting! We're on cqframework version 2.4.0.

@mgramigna mgramigna added bug Something isn't working translator Issues that may involve work with the cql-to-elm translator engine Issues related to the uderlying execution engine and data source labels Jan 26, 2023
@mgramigna
Copy link
Contributor

mgramigna commented Feb 9, 2023

@srankins @nmorasb We investigated this a bit. Before proceeding, I think we need more clarification from @brynrhodes on how the translator is expected to handle accessing properties on a choice type when in the context of a union like this. Take the following CQL:

define "Single":
  [Procedure] P
    return {
      perf: P.performed
    }

define "Union of Same Type":
 ([Procedure] union [Procedure]) P
    return {
      perf: P.performed
    }

define "Union of Different Types":
  ([Procedure] union [ServiceRequest]) ComfortMeasure
    return {
      cmPerformed: ComfortMeasure.performed,
      cmAuthoredOn: ComfortMeasure.authoredOn,
    }

The "Union of Different Types" expression is the only one whose ELM raises concerns (which aligns with what Stan said in his comment above). Here are the corresponding ELM snippets of the elements in the above Tuples:

// from "Single" expression above
{
  "name": "perf",
  "value": {
    "name": "ToValue",
    "libraryName": "FHIRHelpers",
    "type": "FunctionRef",
    "operand": [
      {
        "path": "performed",
        "scope": "P",
        "type": "Property"
      }
    ]
  }
}
// from "Union of Same Type" expression above
{
  "name": "perf",
  "value": {
    "name": "ToValue",
    "libraryName": "FHIRHelpers",
    "type": "FunctionRef",
    "operand": [
      {
        "path": "performed",
        "scope": "P",
        "type": "Property"
      }
    ]
  }
}
// from "Union of Different Types" expression above
{
  "name": "cmPerformed",
  "value": {
    "path": "FHIRHelpers.ToValue(%value)",
    "scope": "ComfortMeasure",
    "type": "Property"
  }
},
{
  "name": "cmAuthoredOn",
  "value": {
    "path": "%value.value",
    "scope": "ComfortMeasure",
    "type": "Property"
  }
},
{
  "name": "coalesced",
  "value": {
    "type": "Coalesce",
    "operand": [
      {
        "type": "Start",
        "operand": {
          "name": "ToInterval",
          "libraryName": "QICoreCommon",
          "type": "FunctionRef",
          "operand": [
            {
              "path": "FHIRHelpers.ToValue(%value)",
              "scope": "ComfortMeasure",
              "type": "Property"
            }
          ]
        }
      },
      {
        "path": "%value.value",
        "scope": "ComfortMeasure",
        "type": "Property"
      }
    ]
  }
}

Note the appearances of FHIRHelpers.ToValue(%value) and %value.value. These are present in the QI Core ModelInfo, but appear to be unwrapped in the other cases but not this one (i.e. FHIRHelpers.ToValue(%value) is translated to an ELM function ref by the translator instead of being used as the literal path).

My suspicion is that the translator needs to be unwrapping these strings with % during translation, and that is not being done for some reason when accessing the properties on the result of a union of different types. As written, the above ELM is telling the engine to look for a literal property named "FHIRHelpers.ToValue(%value)" on a FHIR Procedure resource, which obviously doesn't exist. Hopefully Bryn can clarify

@brynrhodes
Copy link

Yes, that is definitely a bug, the translator is not expanding the mappings in this case. Can you submit this as an issue to the translator?

@mgramigna
Copy link
Contributor

Closing in favor of cqframework/clinical_quality_language#1126

@mgramigna mgramigna removed bug Something isn't working engine Issues related to the uderlying execution engine and data source labels Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translator Issues that may involve work with the cql-to-elm translator
Projects
None yet
Development

No branches or pull requests

4 participants