Optimize CQL library evaluation and consequently measure evaluation (#688)

* First attempt to flip measure and subject looping for population based measure reports.

* Document new unit test failures.

* Abandon previous experiment.  Push up evaluate measure callers to the service layers for single and multi measures and collect data.   Leave old evaluate measure code within processor for the time being as there are still unit test failures otherwise, hopefully limited to single measure evaluation.

* Capture discussion with Justin and requirements but lots of compile errors.

* Put in place changes agreed with Justin and all tests except one are passing.

* Fix CollectData unit test by making resolveParameterMap work with a mutable list.

* Spotless.  Delete some cruft.

* Cleanup and new TODOs.

* Fix compile errors.

* Small tweaks and TODOs.

* Overhaul CompositeEvaluationResultsPerMeasure to include errors as well as successful results, and enhancing it to mutate a given MeasureDef with errors should they exist.  Start cleaning up.  Spotless.

* Cleanup.  Spotless.

* Spotless.

* Handle new MeasureProcessorUtils#setMeasurePeriod, ensure all callers call it correctly, and get rid of duplicate methods.

* Clean up todos and ensure library checking is done in only one place.

* More refactoring and resolve TODOs.

* Resolve sonar warnings.

* Reverse a sonar change that caused errors.  Fix other sonar issues.  Tests pass now.

* Initialize the CqlEngine context at the beginning, pass it to down to the CQL library evaluation, and then to the measure evaluation.

* More cleanup of dead code, commented out code, and dead parameters.

* Fix compile error.  Fix more sonar.  Spotless.  Get rid of unused constructor param.

* Get rid of legacy CQL evaluation and have DSTU3 use the new code.

* Fix sonar and reverse some minor changes.

* Fix DSTU3 tests and resolve last sonar.

* More refactoring to DRY and to remove unused fields.

* Small refactoring for multi-measure service.

* Optimize performance of CompositeEvaluationResultsPerMeasure.

* Spotless.

* Small refactoring.

* Spotless.

* Replace all java.annotation imports with jakarta.annotation.

* Add comment about new approach, as per code review.

* Slight fix to algorithm and TODO for CQL fix.

* Start integrating with new CQL branch with caching optimizations.

* Use the same made-up version of CQL that the CQL branch is using.

* Low risk changes including adding having CompositeEvaluationResultsPerMeasure handle adding multiple errors at once and a TODO to get rid of a record once fixes are complete.

* Grab changes from branch to integrate with CQL changes to validate library identifiers.  Spotless.

* Fix compile error.

* Experiment more with algorithms.   Thus far, if we pass more than one library identifier at a time, we cause definite failures in MultiMeasureServiceTest and intermittent failures on at least one other test if we run the whole module test suite.

* Flip to the new code from the old code in MeasureProcessorUtils.   Add a TODO about what needs to be done to the test to make it pass under the new code.  Add a TODO about another unexpected failure as well.

* Assert that the mulitmeasure "allsubjects" tests still pass but contain InvalidInterval errors.

* Leverage multi-measure error handling code added to CQL.  Spotless.   Still one test failing that I need to figure out.

* Set custom version in the pom for now.

* Add TODO.  Reverse a bunch of logging changes.  Ensure we don't queue up an evaluation result if the library did not evaluate successfully.

* Attempt to set up a successful complex layer library test but no success yet.

* Add unit test.

* Change approach to 1B testing to filter based on meta profiles.  Add meta profiles to all encounters and change IDs accordingly.  Adjust CQLs.  Start trying to assert stratifiers from tests.

* Update to a new stub version ahead of the current master version of clinical-reasoning.  Make R4PopulationBasisValidator more error tolerant, especially if CQL returns Iterators instead of Lists.  Enhance MultiMeasure test util.

* Make final changes to MultiLibEvalComplexCqlTest.  Add back some logging.

* First set of refactoring changes.

* More cleanup.

* Start merging old and new code bases.   Clean up more cruft.

* More refactoring to merge old and new code branches.  More cleanup.

* Small tweaks and integrate with latest CQL branch changes.

* Add temporary logging for db queries.

* Set up complex $evaluate test, which doesn't yet exercise multiple libraries.

* Adapt to new CQL result handling code,  notably for error and exception handling.

* Adapt to new CQL exception handling code.  Comment out RepositoryLoggingProxy.

* Adapt to new CQL library resolution/compilation code.  Add a test that captures the scenario we're trying to optimize for.  Start getting rid of logs.

* Fix test input directory structures to conform to new standard.  Add unversioned local files.

* Fix some TODOs including tests, logging, and javadoc.

* Address rest of TODOs and add another test.

* Sync up with pom versions from master.  Clean up more cruft.

* Change CQL version to 3.28.0.

* Fix tests for new evaluatedMeasures() logic.

* Fix capitalization

---------

Co-authored-by: JP <jonathan.percival@smilecdr.com>
Co-authored-by: Jonathan Percival <jonathan.i.percival@gmail.com>

Author: lukedegruchy

cqf-fhir-benchmark/src/test/java/org/opencds/cqf/fhir/benchmark/measure/r4/Measure.java

@@ -82,7 +82,7 @@ public Given evaluationOptions(MeasureEvaluationOptions evaluationOptions) {
         }
 
         private R4MeasureService buildMeasureService() {
-            return new R4MeasureService(repository, evaluationOptions, measurePeriodValidator, measureServiceUtils);
+            return new R4MeasureService(repository, evaluationOptions, measurePeriodValidator);
         }
 
         public When when() {

cqf-fhir-cql/src/main/java/org/opencds/cqf/fhir/cql/LibraryEngine.java

@@ -14,6 +14,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
@@ -25,6 +26,7 @@
 import org.hl7.fhir.instance.model.api.IBaseParameters;
 import org.opencds.cqf.cql.engine.execution.CqlEngine;
 import org.opencds.cqf.cql.engine.execution.EvaluationResult;
+import org.opencds.cqf.cql.engine.execution.EvaluationResultsForMultiLib;
 import org.opencds.cqf.fhir.cql.engine.parameters.CqlFhirParametersConverter;
 import org.opencds.cqf.fhir.cql.engine.parameters.CqlParameterDefinition;
 import org.opencds.cqf.fhir.utility.CqfExpression;
@@ -325,8 +327,8 @@ public List<IBase> resolveExpression(
         return result;
     }
 
-    public EvaluationResult getEvaluationResult(
-            VersionedIdentifier id,
+    public EvaluationResultsForMultiLib getEvaluationResult(
+            List<VersionedIdentifier> ids,
             String patientId,
             IBaseParameters parameters,
             Map<String, Object> rawParameters,
@@ -336,25 +338,53 @@ public EvaluationResult getEvaluationResult(
             @Nullable ZonedDateTime zonedDateTime,
             CqlEngine engine) {
 
-        if (cqlFhirParametersConverter == null) {
-            cqlFhirParametersConverter = Engines.getCqlFhirParametersConverter(repository.fhirContext());
-        }
+        var cqlFhirParametersConverterToUse = Objects.requireNonNullElseGet(
+                cqlFhirParametersConverter, () -> Engines.getCqlFhirParametersConverter(repository.fhirContext()));
+
         // engine context built externally of LibraryEngine?
-        if (engine == null) {
-            engine = Engines.forRepository(repository, settings, additionalData);
-        }
+        var engineToUse = Objects.requireNonNullElseGet(
+                engine, () -> Engines.forRepository(repository, settings, additionalData));
 
-        var evaluationParameters = cqlFhirParametersConverter.toCqlParameters(parameters);
+        var evaluationParameters = cqlFhirParametersConverterToUse.toCqlParameters(parameters);
         if (rawParameters != null && !rawParameters.isEmpty()) {
             evaluationParameters.putAll(rawParameters);
         }
 
-        return engine.evaluate(
-                new VersionedIdentifier().withId(id.getId()),
+        var versionlessIdentifiers = ids.stream()
+                .map(id -> new VersionedIdentifier().withId(id.getId()))
+                .toList();
+
+        return engineToUse.evaluate(
+                versionlessIdentifiers,
                 expressions,
                 buildContextParameter(patientId),
                 evaluationParameters,
                 null,
                 zonedDateTime);
     }
+
+    public EvaluationResult getEvaluationResult(
+            VersionedIdentifier id,
+            String patientId,
+            IBaseParameters parameters,
+            Map<String, Object> rawParameters,
+            IBaseBundle additionalData,
+            Set<String> expressions,
+            CqlFhirParametersConverter cqlFhirParametersConverter,
+            @Nullable ZonedDateTime zonedDateTime,
+            CqlEngine engine) {
+
+        var evaluationResultsForMultiLib = getEvaluationResult(
+                List.of(id),
+                patientId,
+                parameters,
+                rawParameters,
+                additionalData,
+                expressions,
+                cqlFhirParametersConverter,
+                zonedDateTime,
+                engine);
+
+        return evaluationResultsForMultiLib.getOnlyResultOrThrow();
+    }
 }

cqf-fhir-cql/src/test/java/org/opencds/cqf/fhir/cql/EnginesTest.java

@@ -14,11 +14,11 @@
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.util.BundleBuilder;
+import jakarta.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.stream.StreamSupport;
-import javax.annotation.Nonnull;
 import org.cqframework.cql.cql2elm.StringLibrarySourceProvider;
 import org.cqframework.fhir.npm.NpmPackageManager;
 import org.cqframework.fhir.npm.NpmProcessor;

cqf-fhir-cr-cli/src/main/java/org/opencds/cqf/fhir/cr/cli/command/MeasureCommand.java

@@ -26,10 +26,8 @@
 import org.opencds.cqf.fhir.cr.cli.argument.MeasureCommandArgument;
 import org.opencds.cqf.fhir.cr.cli.command.CqlCommand.SubjectAndResult;
 import org.opencds.cqf.fhir.cr.measure.MeasureEvaluationOptions;
-import org.opencds.cqf.fhir.cr.measure.SubjectProviderOptions;
+import org.opencds.cqf.fhir.cr.measure.common.MeasureProcessorUtils;
 import org.opencds.cqf.fhir.cr.measure.r4.R4MeasureProcessor;
-import org.opencds.cqf.fhir.cr.measure.r4.R4RepositorySubjectProvider;
-import org.opencds.cqf.fhir.cr.measure.r4.utils.R4MeasureServiceUtils;
 import picocli.CommandLine.ArgGroup;
 import picocli.CommandLine.Command;
 
@@ -139,11 +137,7 @@ private static R4MeasureProcessor getR4MeasureProcessor(
         evaluationOptions.setApplyScoringSetMembership(false);
         evaluationOptions.setEvaluationSettings(evaluationSettings);
 
-        return new R4MeasureProcessor(
-                repository,
-                evaluationOptions,
-                new R4RepositorySubjectProvider(new SubjectProviderOptions()),
-                new R4MeasureServiceUtils(repository));
+        return new R4MeasureProcessor(repository, evaluationOptions, new MeasureProcessorUtils());
     }
 
     private void writeJsonToFile(String json, String patientId, Path path) {

cqf-fhir-cr-cli/src/test/java/org/opencds/cqf/fhir/cr/cli/CliTest.java

@@ -327,7 +327,7 @@ void optionsFailure() {
         Main.run(args);
 
         String errOutput = errContent.toString();
-        assertTrue(errOutput.contains("library FluentFunctions loaded, but had errors"));
+        assertTrue(errOutput.contains("Library FluentFunctions loaded, but had errors"));
     }
 
     @Test

cqf-fhir-cr-hapi/src/main/java/org/opencds/cqf/fhir/cr/hapi/config/r4/CrR4Config.java

@@ -48,13 +48,8 @@ public class CrR4Config {
     R4MeasureEvaluatorSingleFactory r4MeasureServiceFactory(
             IRepositoryFactory repositoryFactory,
             MeasureEvaluationOptions evaluationOptions,
-            MeasurePeriodValidator measurePeriodValidator,
-            R4MeasureServiceUtilsFactory r4MeasureServiceUtilsFactory) {
-        return rd -> new R4MeasureService(
-                repositoryFactory.create(rd),
-                evaluationOptions,
-                measurePeriodValidator,
-                r4MeasureServiceUtilsFactory.create(rd));
+            MeasurePeriodValidator measurePeriodValidator) {
+        return rd -> new R4MeasureService(repositoryFactory.create(rd), evaluationOptions, measurePeriodValidator);
     }
 
     @Bean
@@ -90,8 +85,7 @@ ICollectDataServiceFactory collectDataServiceFactory(
             IRepositoryFactory repositoryFactory,
             MeasureEvaluationOptions measureEvaluationOptions,
             R4MeasureServiceUtilsFactory r4MeasureServiceUtilsFactory) {
-        return rd -> new R4CollectDataService(
-                repositoryFactory.create(rd), measureEvaluationOptions, r4MeasureServiceUtilsFactory.create(rd));
+        return rd -> new R4CollectDataService(repositoryFactory.create(rd), measureEvaluationOptions);
     }
 
     @Bean

cqf-fhir-cr-spring/src/main/java/org/opencds/cqf/fhir/cr/spring/measure/MeasureConfiguration.java

@@ -2,10 +2,10 @@
 
 import ca.uhn.fhir.repository.IRepository;
 import org.opencds.cqf.fhir.cr.measure.MeasureEvaluationOptions;
+import org.opencds.cqf.fhir.cr.measure.common.MeasureProcessorUtils;
 import org.opencds.cqf.fhir.cr.measure.common.SubjectProvider;
 import org.opencds.cqf.fhir.cr.measure.dstu3.Dstu3MeasureProcessor;
 import org.opencds.cqf.fhir.cr.measure.r4.R4MeasureProcessor;
-import org.opencds.cqf.fhir.cr.measure.r4.utils.R4MeasureServiceUtils;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.Configuration;
@@ -26,8 +26,7 @@ Dstu3MeasureProcessor dstu3MeasureProcessor(
     R4MeasureProcessor r4MeasureProcessor(
             IRepository repository,
             MeasureEvaluationOptions measureEvaluationOptions,
-            SubjectProvider subjectProvider,
-            R4MeasureServiceUtils measureServiceUtils) {
-        return new R4MeasureProcessor(repository, measureEvaluationOptions, subjectProvider, measureServiceUtils);
+            MeasureProcessorUtils measureProcessorUtils) {
+        return new R4MeasureProcessor(repository, measureEvaluationOptions, measureProcessorUtils);
     }
 }

cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CompositeEvaluationResultsPerMeasure.java

@@ -0,0 +1,110 @@
+package org.opencds.cqf.fhir.cr.measure.common;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.opencds.cqf.cql.engine.execution.EvaluationResult;
+
+/**
+ * Container meant to hold the results or an early and cached CQL measure evaluation that holds
+ * two data points:
+ * <ol>
+ * <li>To hold the results of a measure evaluation, grouped by measure ID, with each measure evaluation grouped by Subject ID</li>
+ * <li>To hold any errors that occurred during the evaluation, grouped by measure ID</li>
+ * </ol>
+ * These data points are not mutually exclusive, meaning a measure may have both successful results and errors.
+ * <p/>
+ * This class also allows the caller to mutate a {@link MeasureDef} with the errors that occurred during the evaluation
+ */
+public class CompositeEvaluationResultsPerMeasure {
+    // The same measure may have successful results AND errors, so account for both
+    private final Map<IIdType, Map<String, EvaluationResult>> resultsPerMeasure;
+    // We may get several errors for a given measure
+    private final Map<IIdType, List<String>> errorsPerMeasure;
+
+    private CompositeEvaluationResultsPerMeasure(Builder builder) {
+
+        var resultsBuilder = ImmutableMap.<IIdType, Map<String, EvaluationResult>>builder();
+        builder.resultsPerMeasure.forEach((key, value) -> resultsBuilder.put(key, ImmutableMap.copyOf(value)));
+        resultsPerMeasure = resultsBuilder.build();
+
+        var errorsBuilder = ImmutableMap.<IIdType, List<String>>builder();
+        builder.errorsPerMeasure.forEach((key, value) -> errorsBuilder.put(key, List.copyOf(value)));
+        errorsPerMeasure = errorsBuilder.build();
+    }
+
+    /**
+     * Retrieves results and populates errors for a given measure.
+     * This method uses direct map lookups for efficient data retrieval.
+     * measureDef will occasionally be prepended with the version, which means we need to parse it into an IIdType which
+     * is too much work, so pass in the measureId directly
+     *
+     * @param measureId the ID of the measure to process
+     * @param measureDef the MeasureDef to populate with errors
+     *
+     * @return a map of evaluation results per subject, or an empty map if none exist
+     */
+    public Map<String, EvaluationResult> processMeasureForSuccessOrFailure(IIdType measureId, MeasureDef measureDef) {
+        var unqualifiedMeasureId = measureId.toUnqualifiedVersionless();
+
+        errorsPerMeasure.getOrDefault(unqualifiedMeasureId, List.of()).forEach(measureDef::addError);
+
+        // We are explicitly maintaining the logic of accepting the lack of any sort of results,
+        // either errors or successes, and returning an empty map.
+        return resultsPerMeasure.getOrDefault(unqualifiedMeasureId, Map.of());
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public static class Builder {
+        private final Map<IIdType, Map<String, EvaluationResult>> resultsPerMeasure = new HashMap<>();
+        private final Map<IIdType, List<String>> errorsPerMeasure = new HashMap<>();
+
+        public CompositeEvaluationResultsPerMeasure build() {
+            return new CompositeEvaluationResultsPerMeasure(this);
+        }
+
+        public void addResults(List<IIdType> measureIds, String subjectId, EvaluationResult evaluationResult) {
+            for (IIdType measureId : measureIds) {
+                addResult(measureId, subjectId, evaluationResult);
+            }
+        }
+
+        public void addResult(IIdType measureId, String subjectId, EvaluationResult evaluationResult) {
+            // if we have no results, we don't need to add anything
+            if (evaluationResult == null || evaluationResult.expressionResults.isEmpty()) {
+                return;
+            }
+
+            var resultPerMeasure =
+                    resultsPerMeasure.computeIfAbsent(measureId.toUnqualifiedVersionless(), k -> new HashMap<>());
+
+            resultPerMeasure.put(subjectId, evaluationResult);
+        }
+
+        public void addErrors(List<? extends IIdType> measureIds, String error) {
+            if (error == null || error.isEmpty()) {
+                return;
+            }
+
+            for (IIdType measureId : measureIds) {
+                addError(measureId, error);
+            }
+        }
+
+        public void addError(IIdType measureId, String error) {
+            if (error == null || error.isBlank()) {
+                return;
+            }
+
+            errorsPerMeasure
+                    .computeIfAbsent(measureId.toUnqualifiedVersionless(), k -> new ArrayList<>())
+                    .add(error);
+        }
+    }
+}