Skip to content

Ld 20251017 populationdef collection optimize #792

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Ld 20251017 populationdef collection optimize #792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e5b233
Move towards merging resource, subject, and subject/resource collecti…
lukedegruchy Oct 17, 2025
4fb7e87
Get rid of standalone subject set and derive subjects from Map.
lukedegruchy Oct 17, 2025
6dd833a
Add TODOs for how to go about removing and replacing functionality fo…
lukedegruchy Oct 17, 2025
98d098c
Fix compile error.
lukedegruchy Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 22 additions & 23 deletions cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ protected Iterable<Object> evaluatePopulationCriteria(
}
}

// LUKETODO: this is where we build out all the resource/subject data
protected PopulationDef evaluatePopulationMembership(
String subjectType, String subjectId, PopulationDef inclusionDef, EvaluationResult evaluationResult) {
// find matching expression
Expand All @@ -151,16 +152,13 @@ protected PopulationDef evaluatePopulationMembership(
int i = 0;
for (Object resource : evaluatePopulationCriteria(
subjectType, matchingResult, evaluationResult, inclusionDef.getEvaluatedResources())) {
// LUKETODO: we'd have to get rid of this and let the other method handle adding the resource
inclusionDef.addResource(resource);
// hashmap instead of set
inclusionDef.addResource(subjectId, resource);

i++;
}
// If SubjectId Added Resources to Population
if (i > 0) {
inclusionDef.addSubject(subjectId);
}
return inclusionDef;
}

Expand Down Expand Up @@ -193,11 +191,11 @@ protected void evaluateProportion(
numerator = evaluatePopulationMembership(subjectType, subjectId, numerator, evaluationResult);
if (applyScoring) {
// remove denominator values not in IP
denominator.getResources().retainAll(initialPopulation.getResources());
denominator.getSubjects().retainAll(initialPopulation.getSubjects());
denominator.retainAllResources(initialPopulation.getResources());
denominator.retainAllSubjects(initialPopulation.getSubjects());
// remove numerator values if not in Denominator
numerator.getSubjects().retainAll(denominator.getSubjects());
numerator.getResources().retainAll(denominator.getResources());
numerator.retainAllSubjects(denominator.getSubjects());
numerator.retainAllResources(denominator.getResources());
}
// Evaluate Exclusions and Exception Populations
if (denominatorExclusion != null) {
Expand All @@ -217,29 +215,29 @@ protected void evaluateProportion(
// Remove Subject and Resource Exclusions
if (denominatorExclusion != null && applyScoring) {
// numerator should not include den-exclusions
numerator.getSubjects().removeAll(denominatorExclusion.getSubjects());
numerator.removeAllSubjects(denominatorExclusion.getSubjects());
numerator.removeOverlaps(denominatorExclusion.getSubjectResources());

// verify exclusion results are found in denominator
denominatorExclusion.getResources().retainAll(denominator.getResources());
denominatorExclusion.getSubjects().retainAll(denominator.getSubjects());
denominatorExclusion.retainAllResources(denominator.getResources());
denominatorExclusion.retainAllSubjects(denominator.getSubjects());
denominatorExclusion.retainOverlaps(denominator.getSubjectResources());
}
if (numeratorExclusion != null && applyScoring) {
// verify results are in Numerator
numeratorExclusion.getResources().retainAll(numerator.getResources());
numeratorExclusion.getSubjects().retainAll(numerator.getSubjects());
numeratorExclusion.retainAllResources(numerator.getResources());
numeratorExclusion.retainAllSubjects(numerator.getSubjects());
numeratorExclusion.retainOverlaps(numerator.getSubjectResources());
}
if (denominatorException != null && applyScoring) {
// Remove Subjects Exceptions that are present in Numerator
denominatorException.getSubjects().removeAll(numerator.getSubjects());
denominatorException.getResources().removeAll(numerator.getResources());
denominatorException.removeAllSubjects(numerator.getSubjects());
denominatorException.removeAllResources(numerator.getResources());
denominatorException.removeOverlaps(numerator.getSubjectResources());

// verify exception results are found in denominator
denominatorException.getResources().retainAll(denominator.getResources());
denominatorException.getSubjects().retainAll(denominator.getSubjects());
denominatorException.retainAllResources(denominator.getResources());
denominatorException.retainAllSubjects(denominator.getSubjects());
denominatorException.retainOverlaps(denominator.getSubjectResources());
}
} else {
Expand All @@ -248,28 +246,29 @@ protected void evaluateProportion(
// * This is why we only remove resources and not subjects too for `Resource Basis`.
if (denominatorExclusion != null && applyScoring) {
// remove any denominator-exception subjects/resources found in Numerator
numerator.getResources().removeAll(denominatorExclusion.getResources());
numerator.removeAllResources(denominatorExclusion.getResources());
numerator.removeOverlaps(denominatorExclusion.getSubjectResources());
// verify exclusion results are found in denominator
denominatorExclusion.getResources().retainAll(denominator.getResources());
denominatorExclusion.retainAllResources(denominator.getResources());
denominatorExclusion.retainOverlaps(denominator.getSubjectResources());
}
if (numeratorExclusion != null && applyScoring) {
// verify exclusion results are found in numerator results, otherwise remove
numeratorExclusion.getResources().retainAll(numerator.getResources());
numeratorExclusion.retainAllResources(numerator.getResources());
numeratorExclusion.retainOverlaps(numerator.getSubjectResources());
}
if (denominatorException != null && applyScoring) {
// Remove Resource Exceptions that are present in Numerator
denominatorException.getResources().removeAll(numerator.getResources());
denominatorException.removeAllResources(numerator.getResources());
denominatorException.removeOverlaps(numerator.getSubjectResources());
// verify exception results are found in denominator
denominatorException.getResources().retainAll(denominator.getResources());
denominatorException.retainAllResources(denominator.getResources());
denominatorException.retainOverlaps(denominator.getSubjectResources());
}
}
if (reportType.equals(MeasureReportType.INDIVIDUAL) && dateOfCompliance != null) {
var doc = evaluateDateOfCompliance(dateOfCompliance, evaluationResult);
// LUKETODO: similarly, we'd have to add the subject as well here
dateOfCompliance.addResource(doc);
}
}
Expand Down Expand Up @@ -299,7 +298,7 @@ protected void evaluateContinuousVariable(
if (applyScoring) {
// verify exclusions are in measure-population
measurePopulationExclusion.getResources().retainAll(measurePopulation.getResources());
measurePopulationExclusion.getSubjects().retainAll(measurePopulation.getSubjects());
measurePopulationExclusion.retainAllSubjects(measurePopulation.getSubjects());
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureProcessorUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ public void continuousVariableObservation(MeasureDef measureDef, CqlEngine conte
measureObservation.getEvaluatedResources(),
groupDef.isBooleanBasis(),
context);
// LUKETODO: this is one place where we directly add a resource with a subject ID
// LUKETODO: this could be tricky, because it could pollute the population of subjectIds
// that we work off
measureObservation.addResource(observationResult);
}
}
Expand Down
46 changes: 42 additions & 4 deletions cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

public class PopulationDef {
Expand All @@ -13,7 +14,6 @@ public class PopulationDef {

protected Set<Object> evaluatedResources;
protected Set<Object> resources;
protected Set<String> subjects;
protected Map<String, Set<Object>> subjectResources = new HashMap<>();

public PopulationDef(String id, ConceptDef code, MeasurePopulationType measurePopulationType, String expression) {
Expand Down Expand Up @@ -56,11 +56,40 @@ public void removeSubject(String subject) {
}

public Set<String> getSubjects() {
if (this.subjects == null) {
this.subjects = new HashSetForFhirResources<>();
return this.getSubjectResources().keySet();
}

public void retainAllResources(Set<Object> resources) {
this.getResources().retainAll(resources);
// LUKETODO: redo this entirely so that we act on the subjectResource EntrySet instead
// getSubjectResources().entrySet().removeIf(entry -> ! containsResource(resources, entry));
}

public void retainAllSubjects(Set<String> subjects) {
this.getSubjects().retainAll(subjects);
}

public void removeAllResources(Set<Object> resources) {
this.getResources().removeAll(resources);
// LUKETODO: redo this entirely so that we act on the subjectResource EntrySet instead
// getSubjectResources().entrySet().removeIf(entry -> containsResource(resources, entry));
}

// LUKETODO: this will help us determine which entry values to keep and which to get rid of:
private boolean containsResource(Set<Object> resources, Entry<String, Set<Object>> entry) {
final Set<Object> resourcesInEntry = entry.getValue();

for (Object resource : resources) {
if (resourcesInEntry.contains(resource)) {
return true;
}
}

return this.subjects;
return false;
}

public void removeAllSubjects(Set<String> subjects) {
this.getSubjects().removeAll(subjects);
}

public void addResource(Object resource) {
Expand All @@ -73,6 +102,15 @@ public Set<Object> getResources() {
}

return this.resources;
// here we want to get the resources from the Map, but make sure
// we wrap that copy in a HashSetForFhirResources so Set comparison
// is what we want
// return new HashSetForFhirResources<>(subjectResources
// .values()
// .stream()
// .flatMap(Collection::stream)
// .filter(Objects::nonNull)
// .collect(Collectors.toUnmodifiableSet()));
}

public String expression() {
Expand Down
1 change: 1 addition & 0 deletions cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.hl7.fhir.r4.model.ResourceType;
import org.junit.jupiter.api.Test;

// LUKETODO: once we get rid of the standalone resources, fix these tests
class PopulationDefTest {

@Test
Expand Down
2 changes: 1 addition & 1 deletion cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ private static PopulationDef buildPopulationRef(Collection<Object> resources) {
null);

if (resources != null) {
resources.forEach(populationDef::addResource);
resources.forEach(res -> populationDef.addResource("subj", res));
}

return populationDef;
Expand Down