sarahmcdougall commented 1 year ago

Summary

Addresses https://github.com/projecttacoma/fqm-execution/issues/182 (single IPP ratio measure numerator/denominator populations should be treated independently in fqm-execution).

New behavior

Fqm-execution currently does not align with HQMF in the fact that ratio measure calculation should always treat numerator and denominator as independent populations. In fqm-execution, this currently only happens when the measure has multiple IPPs.

Fqm-execution now handles the numerator and denominator populations as independent populations for all ratio measures (so, including single IPP).

For example, we should now be able to achieve the following populationResults, meaning the numer is not automatically set to be false if the denom is false:

 "populationResults": [
          {
            "populationType": "initial-population",
            "criteriaExpression": "ipp",
            "result": true,
            "populationId": “ipp-id”
          },
          {
            "populationType": "numerator",
            "criteriaExpression": "numer",
            "result": true,
            "populationId": “numer-id”
          },
          {
            "populationType": "denominator",
            "criteriaExpression": "denom",
            "result": false,
            "populationId": “denom-id”
          }
        ]

Code changes

Overall change: in ClauseResultsBuilder and DetailedResultsBuilder, check that the measure is a ratio measure (rather than checking that it has multiple IPPs) in spots where there is an assumed dependence between the numer and denom populations.

Note that the scoring code can occur at both the group and measure root level, so we should check for the presence of a scoring code in both spots. The measure scoring code is passed in to these functions as a fallback in case the scoring code does not appear at the group level. Open to feedback on a better way to pass in the measure scoring code… the only spot that I could see was passing it in through calculate() since the ClauseResultsBuilder/DetailedResultsBuilder take in the measure group as input rather than the entire measure.

Code changes to focus on:

Update buildPopulationRelevanceMap to check if measure is a ratio measure when deciding whether to set NUMER/NUMEX to false if DENOM is false
Update buildPopulationRelevanceMap to check if measure is a ratio measure when deciding whether to se NUMER/NUMEX to false if DENEX is true
Update handlePopulationValues to do similar checks based on scoring code as those in buildPopulationRelevanceMap
Add unit test in DetailedResultsBuilder for checking numer/denom independence for single IPP ratio measure (plus some rearranging of unit tests - sorry the code diff is really ugly)
Update unit tests in ClauseResultsBuilder to take in the new argument for measure scoring code

Testing guidance

Run unit tests
Run detailed results against patient/measure bundle in https://github.com/projecttacoma/fqm-execution/issues/182 - on the main branch the numerator should false out, but on this branch the numerator should stay as “true” in the populationResults
Run detailed results against the same patient/measure bundle but change the scoring code to “proportion” instead of “ratio” - the numerator should false out since it is dependent with the denominator (since this only applies to ratio measures)

github-actions[bot] commented 1 year ago

Coverage report

St.^{:grey_question:}	Category	Percentage	Covered / Total
🟢	Statements	84.98% (+0.02% 🔼)	1986/2337
🟡	Branches	73.6% (-0.01% 🔻)	1745/2371
🟢	Functions	87.1% (+0.03% 🔼)	351/403
🟢	Lines	85.26% (+0.02% 🔼)	1914/2245

Show files with reduced coverage 🔻

|

St.^{:grey_question:}

| File | Statements | Branches | Functions | Lines | | :----------------------------------------------------------------------------------------------------------------------------------------------------: | :------------------------------------------------------------------------------------- | :--------- | :------------------------------------------------------------- | :-------- | :---- | | 🟡 |

`...` / ClauseResultsBuilder.ts

| 72.84% |

61.05% (-0.43% 🔻)

| 86.49% | 72.4% |

Test suite run success

363 tests passing in 28 suites.

Report generated by 🧪jest coverage report action from e0afb9cf4e6503667dfd43fc229a65ae8509d573

sarahmcdougall commented 1 year ago

One overall thing I noticed though was there don't seem to be any unit tests for testing numer/denom independence when there is only 1 ipp and its a ratio measure. This should maybe be something we add unless it's there and I just missed it.

They are there (they probably got overshadowed by the rest of the code diff since I made a lot of little changes surrounding them) - the unit tests are in a describe block at the bottom of DetailedResultsBuilder.test.ts: 'handlePopulationValues for group with single IPP - ratio measure'

projecttacoma / fqm-execution

Independent numer/denom for ratio measures #190