Reporting module: Row per diagnosis report

mksrom · August 5, 2017, 5:52am

Hi @mseaton! I have report on my list that would be a row per diagnosis table (there is ~200 of them) and each row would have displayed the number of patients/visits with this diagnosis, grouped by patient age and gender.

Something like:

| ICD-10 |  0-1 month (M) | 0-1 month (F) | 1-12 months (M) | 1-12 months (M) |
| A00.9  |    2           |               |                 |   1             |
| A01    |                |               |                 |                 |
| A06    |    1           |     4         |                 |                 |
| A09.2  |                |               |    5            |                 |
...

Do I need to create a new DiagnosisDataSetDefinition? Any existing example out there that you can think of?

Thanks

Romain

hpardess · August 7, 2017, 9:17am

I need similar report but I tried to write a query and then designed the template in XReports

SELECT * FROM (
(SELECT 
(SELECT `name` FROM `concept_name` 
WHERE `voided` = 0 AND `locale` IN ('en','pa') AND `concept_id` = diagnose.`value_coded`  
AND `concept_name_type` = 'FULLY_SPECIFIED' ORDER BY `locale` DESC LIMIT 1) AS DiagnoseName, 'Coded' AS 'Type', COUNT(*) AS 'Total Patients'

FROM encounter AS visit_note_encounter, obs AS diagnose, obs AS diagnose_type, person, person_name, patient_identifier
WHERE visit_note_encounter.voided = 0 
AND visit_note_encounter.encounter_type = 5
AND DATE(visit_note_encounter.encounter_datetime) BETWEEN :startDate AND :endDate
AND diagnose.voided = 0	
AND diagnose.encounter_id = visit_note_encounter.encounter_id
AND diagnose.concept_id = 1284 -- it is coded diagnose concept - PROBLEM LIST
AND diagnose_type.encounter_id = visit_note_encounter.encounter_id
AND diagnose.obs_group_id = diagnose_type.obs_group_id AND diagnose_type.concept_id = 159946 
AND diagnose_type.value_coded = 159943

AND person.person_id = person_name.person_id 
AND person.person_id = patient_identifier.patient_id
AND person.voided=0 
AND person_name.voided=0 
AND patient_identifier.voided=0
AND person_name.preferred=1 
AND patient_identifier.preferred=1 
AND patient_identifier.identifier_type=3
AND patient_identifier.patient_id = visit_note_encounter.patient_id
GROUP BY DiagnoseName
ORDER BY DiagnoseName ASC)

UNION ALL

(SELECT diagnose.value_text AS DiagnoseName, 'Non-Coded' AS 'Type', COUNT(*) AS 'Total Patients'

FROM encounter AS visit_note_encounter, obs AS diagnose, obs AS diagnose_type, person, person_name, patient_identifier
WHERE visit_note_encounter.voided = 0 
AND visit_note_encounter.encounter_type = 5
AND DATE(visit_note_encounter.encounter_datetime) BETWEEN :startDate AND :endDate
AND diagnose.voided = 0
AND diagnose.encounter_id = visit_note_encounter.encounter_id
AND diagnose.concept_id = 161602 -- it is coded diagnose concept - Diagnosis or problem, non-coded
AND diagnose_type.encounter_id = visit_note_encounter.encounter_id
AND diagnose.obs_group_id = diagnose_type.obs_group_id AND diagnose_type.concept_id = 159946 
AND diagnose_type.value_coded = 159943

AND person.person_id = person_name.person_id 
AND person.person_id = patient_identifier.patient_id
AND person.voided=0 
AND person_name.voided=0 
AND patient_identifier.voided=0
AND person_name.preferred=1 
AND patient_identifier.preferred=1 
AND patient_identifier.identifier_type=3
AND patient_identifier.patient_id = visit_note_encounter.patient_id
GROUP BY DiagnoseName
ORDER BY DiagnoseName ASC)
) t

Count of Patients based Primary Diagnose in Interval

here is the sample report I have designed:

mseaton · August 7, 2017, 12:41pm

@mksrom, a few thoughts.

You could do as @hpardess indicates and use SQL (or a custom data set definition) to produce the data of interest, and just use that.
If the numbers in your report can be “total patients with diagnosis” in each category, rather than “total diagnoses” in each category (eg. a given patient is not intended to be double-counted), then you could use a CohortCrossTabDataSetDefinition. Your 'rows" would be CodedObsCohortDefinition instances configured with “Diagnosis” as the Question, and different Concepts configured in the valueList property. Your ‘columns’ would be a CompositionCohortDefinition of “GenderCohortDefinition” and “AgeCohortDefinition”.
Similar to 2, you could use a CohortIndicatorAndDimensionDataSetDefinition, where each indicator would be the number of patients with a given diagnosis, and the dimensions would be age range and gender.
You could use an ObsDataSetDefinition to produce one row per obs, with Age and Gender columns, similar to what you did with the VisitDataSetDefinition. You would then need to figure out how to manipulate that DataSet to do the aggregation - maybe in the renderer.

Happy to brainstorm more - just a few options that might work for you (or not).

Mike

mksrom · August 15, 2017, 3:43pm

@hpardess, thanks for sharing your SQL query here. For the moment I will try to not use SQL as much as possible. I find it too difficult to maintain and test.

mksrom · August 15, 2017, 3:49pm

@mseaton option #2 seems the right one for me .

Let’s make the problem simpler a bit for the start and display only one diagnosis (as one line) and the number of patients matching a given gender for that line (and not use the CompositionCohortDefinition yet):

Something like:

| Diagnosis     | Males | Females |
|---------------|-------|---------|
| Encephalocele |  0    |    1    |

This is the intersection of the CodedObsCohortDefinition (set with the ‘Diagnosis’ question and with the ‘encephalocele’ concept as valueList) and two GenderCohortDefinition

Here is what I am doing now:

ReportDefinition rd = new ReportDefinition();
rd.setParameters(getParameters());
...

CohortCrossTabDataSetDefinition OPDConsult = new CohortCrossTabDataSetDefinition();

CodedObsCohortDefinition diag = new CodedObsCohortDefinition();
diag.addParameter(new Parameter("onOrAfter", "On Or After", Date.class));
diag.addParameter(new Parameter("onOrBefore", "On Or Before", Date.class));
diag.setOperator(SetComparator.IN);
diag.setQuestion(conceptService.getConceptByUuid("81c7149b-3f10-11e4-adec-0800271c1b75"));
diag.setValueList(Arrays.asList(conceptService.getConceptByUuid("f35731b9-4e14-11e4-8a57-0800271c1b75")));

Map<String, Object> parameterMappings = new HashMap<String, Object>();
parameterMappings.put("onOrAfter", "${startDate}");
parameterMappings.put("onOrBefore", "${endDate}");

// Add the 'Encephalocele' row
OPDConsult.addRow(concept.getDisplayString(), diag, parameterMappings);

// Add the 'Males' column
GenderCohortDefinition males = new GenderCohortDefinition();
males.setMaleIncluded(true);
OPDConsult.addColumn("Males", males, null);

// Add the 'Females' column
GenderCohortDefinition females = new GenderCohortDefinition();
females.setFemaleIncluded(true);
OPDConsult.addColumn("Females", females, null);

rd.addDataSetDefinition("Outpatient Consultation", Mapped.mapStraightThrough(OPDConsult));

This returns the following table:

Even though one female patient has an ‘encephalocele’ diagnosis, the table shows empty.

While debugging I can see that the diag CodedObsCohortDef is correctly returning this matching person, ie [70].

However males and females GenderCohortDefs are returning an empty array.

(Q) Am I using them correctly?

(Q) In the example above the report parameters are not taken into account at all. Should I pass the mapping a bit differently? (I notice that the ‘addColumn’ method does not require a ‘String’ as parameters but rather directly parameter map)

Thanks

Romain

mseaton · August 15, 2017, 4:38pm

@mksrom - this looks right. I’m not sure why the males and females would be showing 0. What happens if you add a 3rd column for “All Patients”? What happens if you remove the Males and Females columns? What happens if you remove the row definition?

Regarding the parameters, it does seem like you are not appropriately adding startDate and endDate parameters to your OPDConsult DSD. You’ll need to do that.

Mike

mksrom · August 16, 2017, 6:04am

With no row and with the AllPatientsCohortDefinition as well as a GenderCohortDefinition set with all genders (males, females, unknown):

With the ‘Encephalocele’ row and only with AllPatientsCohortDefinition:

mksrom · August 16, 2017, 12:54pm

Looking at the first problem, ie, GenderCohortDefintion returns empty

I noticed that when CohortDataSetEvaluator is run, the Cohort.intersect(rowCohort, colCohort) method returns an empty Cohort.

Digging into Cohort.intersect(a,b) method, I find that this is the line that empties the returned Cohort:

ret.getMemberships().retainAll(b.getMemberships())

a and b collections are containing indeed 2 different CohortMembership objects:

But their value is the same patient:

(Q) Should 2 CohortMembership objects with the same patient be considered equal?

Note: The described behavior is only the case for the GenderCohortDefinition evaluation. During the AllPatientsCohortDefinition evaluation, the a.getMemberships() and b.getMemberships() are containing the exact same objects, therefore the returned Cohort contains patient 70.

mksrom · August 16, 2017, 12:56pm

I forgot to mention that I am running on Core 2.1.0

mseaton · August 16, 2017, 2:26pm

Good detective work. This definitely sounds like the problem. We don’t even have a profile in the reporting module for testing against 2.1 yet - we should add that in. I’m guessing things will fail badly around this due to the changes to the Cohort API.

I can look into it a bit more, but also hope @darius and @wyclif and others who were involved with this change to Cohort Membership can weigh in, as it sounds to me like it is a bug there perhaps. If this fails, that tells me that the Cohort.intersect(Cohort, Cohort) method also likely fails. I don’t see any implementation of hashCode or equals in the CohortMembership class - that might be the issue.

Thoughts? Also cc @raff and @dkayiwa

Mike

mseaton · August 16, 2017, 3:50pm

To follow-up on this, I have added a 2.1 test profile, which you can run using mvn clean install -P2.1, which has failing tests now for these cases.

Mike

mseaton · August 16, 2017, 3:58pm

Further follow-up. I updated to the latest from core, and add this to the end of CohortTest:

@Test
public void intersect_shouldRetainMembershipsForTheSamePatient() throws Exception {
    Cohort cohortOne = new Cohort();
    cohortOne.addMember(7);
    cohortOne.addMember(8);
    cohortOne.addMember(9);

    Cohort cohortTwo = new Cohort();
    cohortTwo.addMember(7);

    Cohort cohortIntersect = Cohort.intersect(cohortOne, cohortTwo);
    Assert.assertEquals(1, cohortIntersect.size());
    assertTrue(cohortIntersect.contains(7));
}

This test fails. The resulting cohortIntersect is empty. I view this as a bug in core. @wyclif / @darius?

Mike

lluismf · August 16, 2017, 9:07pm

The implementation of compareTo gives a hint:

	@Override
	public int compareTo(CohortMembership o) {
		return this.getPatient().getPatientId() - o.getPatient().getPatientId();
	}

According to the Java API:

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals.

darius · August 16, 2017, 10:23pm

@mseaton, the behavior of Cohorts changes in openmrs-core 2.1.0 so that membership now also encompasses start/end dates.

Thus the behavior of Cohort.intersect changes (see this comment here: https://issues.openmrs.org/browse/TRUNK-4906?focusedCommentId=235565&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-235565 )

I think there’s supposed to be a one-line replacement which would make things work for the reporting module against newer versions of openmrs-core, though I don’t remember what it ended up being called.

One reason that your test may fail in core 2.1.0+ is that cohort.addMember(Integer) ends up setting the cohort membership start date to new Date(). Though actually I would expect this test to pass most of the time except when a clock tick happens during the test…

One could argue that the correct behavior if you do new CohortMembership(Integer) should be to leave the start date null. I wouldn’t object to making that change.

It’s also true that since we define compareTo we should be defining equals and hashCode.

(In the bigger picture, I guess that we knew that this code would break the reporting module, and it’s not super well tested in general, so it would be great if someone has a chance to dive into figuring out whether there’s an easy fix and where it needs to happen.)

mseaton · August 17, 2017, 1:54am

Thanks for the detailed reply @darius. A few thoughts:

I see that comment (and it makes me feel better to see how much review this got). Still, I don’t think the implementation of these methods is correct. A Cohort is now essentially a TreeSet of CohortMembership objects. Intersect and Union operations will operate based on equality of a CohortMembership, right? CohortMembership inherits it’s equals and hashcode implementation from BaseOpenmrsObject, which simply utilizes the uuid. Assuming that, how would any union or intersection work? Isn’t this a bug?

Yes, I would certainly argue that. I honestly don’t know what the use cases for Cohort Membership with start and end dates are (I understand the principle, I don’t know where this is used or needed practically speaking in OpenMRS today), so I may not see the argument for setting this to new Date(), but it would certainly seem to me that setting these new date fields to null by default, and then ensuring that all legacy cohort behavior acted in a completely backwards-compatible way if all date fields are null, would make a lot of sense.

Yeah. At least for the Cohort.intersect method that I was testing, there were basically no existing tests unfortunately. I’m going to look to see if this is relatively easy to code around in the reporting module, but I also wonder if we were to agree on the above, and change the default behavior to set dates to null, and to change the equals/hashcode methods of CohortMembership to look at the meaningful fields, that things might actually work correctly again.

Mike

mseaton · August 22, 2017, 3:06am

To tie up this thread a little bit, I have made a few commits to the reporting module so that it now builds successfully against 1.9, 1.10, 1.11, 1.12, 2.0, and 2.1 profiles, without any changes to core (see https://issues.openmrs.org/browse/REPORT-829). I have also added a ticket for relaxing the non-null constraint on cohort member start dates here: https://issues.openmrs.org/browse/TRUNK-5211

Mike

mksrom · August 22, 2017, 4:54am

Thanks for looking into this @mseaton , I will pull these changes and try again.

mksd · September 20, 2017, 8:13am

Hey @mseaton,

We have an issue at runtime against Core 2.1 (Bahmni):

NoSuchMethodError: EvaluatedCohort.addMember(Ljava/lang/Integer;)V

And indeed since Core 2.1 this method’s signature has changed:

Pre 2.1 it returned nothing (the ‘V’ return type.)
Since 2.1 it returns a boolean.

Since this method signature has changed on Cohort, does that mean that some sort of CohortCompatibility class will be needed? Or what would be your suggested workaround?

@darius, how important was it to change the return type of that method from Cohort? This is the exact change in its own commit. I assume that it was necessary, but just in case it wasn’t, perhaps this could be reverted?

mseaton · September 20, 2017, 2:22pm

@mksd, it’s a good question - I have never encountered this but it does seem like a backwards-incompatibility introduced in core.

As an aside, I think the meaning of Cohort has significantly changed in recent releases with the introduction of Cohort Membership. We might actually consider breaking the inheritance between Cohort in core and PatientIdSet / EvaluatedCohort in reporting, since reporting still sees Cohorts as simple sets of patient ids. But I haven’t looked at the full implications of this.

Not sure the best approach to getting around your current problem, but interested in hearing ideas.

Mike

mksd · September 20, 2017, 5:08pm

Oblivious of what the Reporting module does with Cohort I’m afraid that I am not going to come up with mind blowing ideas at this juncture

Simply something along the lines of tracking invocations of Cohort.addMember(Integer), and wrap them in a call of a new function added to CohortUtil. And the compatibility work would be encapsulated inside CohortUtil. Like:

public class CohortUtil {
  ...
  public static boolean addMember(Cohort cohort, Integer memberId) {
    CohortCompatibility cp = ... 
    return cp.addMember(cohort, memberId);
  }
  ...
}