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

Syntax issue ( allowable, but causes error from fqm-execution ) #222

Closed
gregory-akins opened this issue Apr 12, 2023 · 7 comments
Closed
Labels
added to backlog Task has been added to the team's backlog and will be prioritized accordingly engine Issues related to the uderlying execution engine and data source translator Issues that may involve work with the cql-to-elm translator

Comments

@gregory-akins
Copy link

Summary

CQL contains a strange, but allowable syntax

or "Macular edema absent (situation)" in MacularEdemaAbsentNotCommunicated.reasonCode

When that clause is included in the CQL, fqm-execution returns an error ( e is undefined )

Expected Behavior

parsed & executed successfully

Stan Rankins reports

Greg Akins
1 day ago
Do you think the syntax “should” work, or it should through an error from CQL-ELM-Translator (which it’s not)?

Stan Rankins
1 day ago
That's what they are being told to do by Bryn - https://github.com/cqframework/CQL-Formatting-and-Usage-Wiki/wiki/Authoring-Patterns---QICore-v4.1.1#communication-not-done

Version or Commit

1.0.8

Inputs (e.g. Measure Bundle, Patient Bundle, CQL Library)

bundleAndTestcas.zip

Relevant Calculation Options (e.g. Measurement Period, meta.profile Validation)

cql-to-elm Version Used for Measure Logic Translation (if known)

CQFramework 2.7.0

Any Additional Info

@mgramigna
Copy link
Contributor

mgramigna commented Apr 12, 2023

Thanks for the issue. It looks like cql-execution is failing to even construct a valid expression based on the ELM provided, so the error is happening before it even tries any execution. Thanks for providing the detailed inputs, I think this will require digging into the ELM and the CQL to see what’s going on. We will look at this. Let me know if you come across anything interesting in the meantime.

The e is undefined message that happens from the minified JavaScript is not super helpful unfortunately. I linked it to my local TypeScript files and got the following:

image

It's failing on the constructor of the Expression, which tries to recursively build the internal cql-execution model based on the expression tree defined in the ELM. Specifically, it's trying to access operand on something that is undefined. That should be enough for us to start poking at at the very least. Will keep you updated with progress.

@mgramigna mgramigna added added to backlog Task has been added to the team's backlog and will be prioritized accordingly translator Issues that may involve work with the cql-to-elm translator engine Issues related to the uderlying execution engine and data source labels Apr 20, 2023
@birick1
Copy link
Contributor

birick1 commented May 4, 2023

Related: cqframework/cql-execution#296

@birick1
Copy link
Contributor

birick1 commented May 4, 2023

There will be discussion around this item at the May 2023 HL7 Connectathon

@birick1
Copy link
Contributor

birick1 commented May 8, 2023

Discussed with @brynrhodes at the May 2023 HL7 Connectathon.

Resolving this issue requires an update in both the translator and cql-execution.

For the translator, an update to the QICore modelinfo file is required and a ticket has been filed at cqframework/clinical_quality_language#1168.

Once the translator is updated, a resolution for valueset expressions can be worked in cql-execution. Track cqframework/cql-execution#296. fqm-execution will then incorporate the updated `cql-execution'.

@birick1
Copy link
Contributor

birick1 commented May 8, 2023

@gregory-akins: Originated from MADiE Issue Tracker 1455

@mgramigna
Copy link
Contributor

Just to follow up on this one, after a lot of discussion in the past couple weeks, it has been determined that the authoring pattern for CommunicationNotDone now explicitly discourages use of the "<direct reference code>" in <alias>.reasonCode syntax:

NOTE: The positive counterpart for this statement (Macular Edema Absence Communicated) uses a direct-reference code, and ideally the negative statement would as well. However, at this time, direct-reference codes cannot be used as the terminology target of a retrieve of a negation profile. The workaround for this issue is to create a value set with the single required code and use the value set negation pattern.

The updated syntax of that authoring pattern is supported as is in fqm-execution only if a reasonCode is directly present on the data. If the data is using the qicore-notDoneValueSet extension, this will not work in fqm-execution until cqframework/cql-execution#296 is addressed. Our team will discuss priorities around that cql-execution issue and how best to move forward.

One key assumption that we've discussed as well is that any canonical to a ValueSet referenced using the notDoneValueSet extension must be resolved prior to doing execution. It should be a ValueSet that is already included in the CQL for the measure logic. So fqm-execution will expect any ValueSet referenced in the canonical of that extension to have already been provided to the engine.

I might suggest closing this issue, as the specific syntax that caused it is no longer of concern, but I will wait for input from others.

@mgramigna
Copy link
Contributor

Follow cqframework/cql-execution#296 for more progress on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
added to backlog Task has been added to the team's backlog and will be prioritized accordingly engine Issues related to the uderlying execution engine and data source translator Issues that may involve work with the cql-to-elm translator
Projects
None yet
Development

No branches or pull requests

3 participants