Skip to content
New issue

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

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

Already on GitHub? # to your account

Cannot read properties of null (reading 'FHIRHelpers') #277

Closed
JSRankins opened this issue Sep 26, 2022 · 9 comments · Fixed by #282
Closed

Cannot read properties of null (reading 'FHIRHelpers') #277

JSRankins opened this issue Sep 26, 2022 · 9 comments · Fixed by #282

Comments

@JSRankins
Copy link

JSRankins commented Sep 26, 2022

@cmoesel and @brynrhodes Good morning gents. I've run into an error using FHIRHelpers and the QI-Core version of MATGlobalCommonFunctions library as it centers around the following function:

define fluent function hospitalArrivalTime(TheEncounter Encounter ): start of First( ( "Hospitalization Locations"(TheEncounter) ) HospitalLocation sort by start of period ).period

The issue arises when there is a missing location reference within an ED Encounter resource even though there is a location reference within the Inpatient Encounter resource.

I'm receiving the following message:

Cannot read properties of null (reading 'FHIRHelpers')

Problem with CQL logic or execution-engine? Thoughts?

The CQL referencing the hospitalArrivalTime function is here.

Patient-bundle:
{ "resourceType": "Bundle", "id": "denom-pass-EncounterLt2Days", "meta": { "versionId": "3", "lastUpdated": "2022-09-14T12:38:39.889+00:00" }, "type": "collection", "entry": [ { "fullUrl": "609bde3598086b0a16d79fc6", "resource": { "resourceType": "Patient", "id": "609bde3598086b0a16d79fc6", "meta": { "profile": [ "http://hl7.org/fhir/us/qicore/StructureDefinition/qicore-patient" ] }, "text": { "status": "generated", "div": "<div xmlns=\"http://www.w3.org/1999/xhtml\"><div class=\"hapiHeaderText\">LocationPeriodStartTimeMissing <b>MSRPOPLEXSTRAT2PASS </b></div><table class=\"hapiPropertyTable\"><tbody><tr><td>Identifier</td><td>8065dc8d26797064d8766be71f2bf020</td></tr><tr><td>Date of birth</td><td><span>10 February 1954</span></td></tr></tbody></table></div>" }, "identifier": [ { "type": { "coding": [ { "system": "http://terminology.hl7.org/CodeSystem/v2-0203", "code": "MR" } ] }, "system": "http://myGoodHealthcare.com/MRN", "value": "8065dc8d26797064d8766be71f2bf020" } ], "active": true, "name": [ { "use": "usual", "family": "IPPass", "given": [ "Encounter Less than 2 Days" ] } ], "gender": "male", "birthDate": "1954-02-10" } }, { "fullUrl": "5c6c61ceb84846536a9a98f9", "resource": { "resourceType": "Encounter", "id": "5c6c61ceb84846536a9a98f9", "status": "finished", "class" : { "system" : "http://terminology.hl7.org/CodeSystem/v3-ActCode", "code" : "IMP", "display" : "inpatient encounter" }, "type": [ { "coding": [ { "system": "http://snomed.info/sct", "code": "183452005", "display": "Emergency hospital admission (procedure)" } ] } ], "subject": { "reference": "Patient/609bde3598086b0a16d79fc6" }, "period": { "start": "2012-07-15T08:00:00+00:00", "end": "2012-07-16T09:00:00+00:00" }, "length": { "value": 1.0, "unit": "days" }, "location" : [ { "location" : { "reference" : "Location/4989ju789fn93bvy562loe87c", "display" : "Holy Family Hospital Inpatient" }, "period": { "start": "2012-07-15T08:00:00+00:00", "end": "2012-07-16T09:00:00+00:00" } } ] } }, { "fullUrl": "9dju7njdn764mdjy6dm92nje", "resource": { "resourceType": "Encounter", "id": "9dju7njdn764mdjy6dm92nje", "status": "finished", "class" : { "system" : "http://terminology.hl7.org/CodeSystem/v3-ActCode", "code" : "EMRGONLY", "display" : "Emergency only" }, "type": [ { "coding": [ { "system": "http://snomed.info/sct", "code": "4525004", "display": "Emergency department patient visit (procedure)" } ] } ], "subject": { "reference": "Patient/609bde3598086b0a16d79fc6" }, "period": { "start": "2012-07-14T23:00:00+00:00", "end": "2012-07-15T07:30:00+00:00" }, "length": { "value": 1.0, "unit": "days" } } }, { "fullUrl": "489juh6757h87j03jhy73mv7", "resource": { "resourceType" : "Location", "id" : "489juh6757h87j03jhy73mv7", "meta" : { "profile" : [ "http://hl7.org/fhir/us/qicore/StructureDefinition/qicore-location" ] }, "identifier" : [ { "use" : "official", "system" : "http://holycrosshospital.com/location", "value" : "489juh6757h87j03jhy73mv7" } ], "status" : "active", "name" : "South Wing, second floor" } }, { "fullUrl": "4989ju789fn93bvy562loe87c", "resource": { "resourceType" : "Location", "id" : "4989ju789fn93bvy562loe87c", "meta" : { "profile" : [ "http://hl7.org/fhir/us/qicore/StructureDefinition/qicore-location" ] }, "identifier" : [ { "use" : "official", "system" : "http://holycrosshospital.com/location", "value" : "4989ju789fn93bvy562loe87c" } ], "status" : "active", "name" : "North Wing, second floor" } } ] }

@JSRankins
Copy link
Author

Just checking status of this ticket...

@cmoesel
Copy link
Member

cmoesel commented Oct 18, 2022

Hi @JSRankins -- it seems that your link to the CQL in the post above is 404. Did that file move?

@JSRankins
Copy link
Author

Hi @cmoesel. Yes. My apologies. Active in that repo right now, and I had a similarly named library. So I changed it just yesterday. It is here.

@cmoesel
Copy link
Member

cmoesel commented Oct 20, 2022

Thank you, @JSRankins. I don't know exactly how you're running this, but I was able to create a simple test project that reproduces the issue: ecqm-test.zip. The README contains instructions for running it.

I forgot to put this in the README, but note that I had to add the meta.profile to the Encounters in the Patient bundle since I'm running this in trusted environment mode.

I'll try to take a closer look at what's going on tomorrow.

@JSRankins
Copy link
Author

Thanks @cmoesel. My apologies. I was in a hurry when I wrote this up. I was in a lower level MADiE environment when I ran into the issue. If you want version of Translator and fqm-execution that instance of tool was on when I encountered the issue, I'll need to do some digging, but I can let you know once I have those details. Next time (if there is one), I'll put those details in the ticket.

@cmoesel
Copy link
Member

cmoesel commented Oct 21, 2022

Don't worry about it, @JSRankins. Since I was able to reproduce this using the latest versions of our libraries, I'm at a good point to be able to investigate (and hopefully fix) it.

@p9g
Copy link

p9g commented Oct 21, 2022

@JSRankins the Encounters in your test case do not have the meta.profile for QI-Core Encounter
Sorry I missed the @cmoesel comment above when I added this comment

@JSRankins
Copy link
Author

@p9g, base FHIR doesn't require meta. In fact, it isn't even required in QI-Core. Understandably, it is needed when operating in trusted environment. If you see @cmoesel's comment above, he added meta to the Encounter resource instances, and the issue still occurs.

cmoesel added a commit that referenced this issue Oct 21, 2022
Added a broken test showing that when you call Context.childContext(null), it results in null context_values instead of the intended default empty obect ({}).
@cmoesel
Copy link
Member

cmoesel commented Oct 22, 2022

@JSRankins - I've found the problem and submitted a PR with the fix (#282). Thanks for reporting this!

cmoesel added a commit that referenced this issue Oct 26, 2022
…ect (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest
birick1 pushed a commit that referenced this issue Jan 15, 2023
…ect (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest
mgramigna pushed a commit that referenced this issue Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
cmoesel added a commit that referenced this issue Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Mar 28, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Mar 29, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
mgramigna pushed a commit that referenced this issue Apr 21, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
cmoesel added a commit that referenced this issue May 4, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
cmoesel added a commit that referenced this issue May 4, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants