Temporal annual distribution functionality by alex-odysseus · Pull Request #271 · OHDSI/FeatureExtraction

alex-odysseus · 2024-08-28T20:16:02Z

Supporting OHDSI/WebAPI#2331 to enable temporal annual distribution extending the existing temporal functionality

Temporal annual distribution will be effective for Feature Analyses specified in newly added PrespecTemporalAnnualAnalysis.csv

The idea of the functionality is to extract an underlying year for a Covariate of interest so that it can be easily grouped by in Cohort Characterization result visualization in WebAPI / ATLAS

Minor refactoring using Stream and StreamSupport classes

Standard Java formatting applied in a few places (hopefully acceptable)

Referencing SqlRender 1.18.1 in pom.xml

Targeting feature for 3.7.0 (for an unknown reason there is still 3.5.1-SNAPSHOT in 'develop' pom.xml even though 3.6.0 has been officially released

Referencing SqlRender 1.18.1 Version 3.7.0-SNAPSHOT as 3.6.0 has been released even though there were no changes to pom.xml Merge remote-tracking branch 'remotes/origin/ATL-10' into develop # Conflicts: # inst/java/featureExtraction-3.3.0-SNAPSHOT.jar # java/org/ohdsi/featureExtraction/FeatureExtraction.java

alex-odysseus · 2024-08-28T20:19:11Z

@anthonysena Anthony, I can't assign reviewers, please do so

anthonysena

@alex-odysseus - I've provided some feedback on this PR for your review. I have not reviewed the other associated PRs so I'm unsure of the impact this proposed change would have on this feature. That said, here is my feedback:

We need to make sure any enhancements to FeatureExtraction are done so that users from the R side can access this functionality. As it currently stands, I don't see a way that an R user could invoke this functionality.
I think we can simplify the way in which this feature is invoked by simply passing in an option to perform the annual distribution as noted in the comments.
All R command checks and unit tests must pass for this to be considered in a state for merging into the develop branch.

Thanks!

anthonysena · 2024-09-09T13:34:13Z

inst/sql/sql_server/DomainConcept.sql

 	row_id,
 	1 AS covariate_value 
 }
+{@temporal_annual} ? {, event_year}


The temporal_annual appears to be the way in which a user would indicate to FeatureExtraction to compute the annual distribution yet this parameter is never exposed to the user best I can tell. In reviewing the code, the approach appears to be: specify the analysis IDs in the PrespecTemporalAnnualAnalysis.csv and then the Java layer will then inject the temporal_annual parameter value.

PrespecTemporalAnnualAnalysis.csv contains Feature Analysis entries (for instance, demographics related) which were intentionally omit in a new CSV file being created

In OHDSI/SkeletonCohortCharacterization#46 CCBuilder.buildSettings there are separate Maps for that reason not to mix up

I would propose to leave it separate

anthonysena · 2024-09-10T14:28:44Z

@alex-odysseus thanks for reviewing this with me on Atlas/WebAPI WG call. Here are some specific tasks from our discussion:

Remove inst/csv/PrespecTemporalAnnualAnalysis.csv as it is redundant with the analyses already defined in the package.
Add an annualPrevalence parameter to the getDbCovariateData function to expose the ability to compute the annual prevalence. Make sure this is passed to the Java layer so it can properly pass this to the SQL layer...
Tidy up the Java class based on the feedback above. For the createQuerySql function of that class, expose an annualPrevalence parameter to enable passing in the parameter from R and the SkeletonCohortCharacterization layers respectively.

chrisknoll · 2025-02-11T17:37:58Z

The main outstanding issue is that R-CMD-Check is not passing, so we can merge this in once the errors are cleared.

anthonysena · 2025-02-11T17:40:58Z

I'll also note that this will be a Java-only enabled feature until we find a use-case for this in the R side.

…-annual-distribution # Conflicts: # java/org/ohdsi/featureExtraction/FeatureExtraction.java # pom.xml

anthonysena · 2025-02-18T18:53:10Z

java/org/ohdsi/featureExtraction/FeatureExtraction.java

 			temporalSequence = jsonObject.getBoolean(TEMPORAL_SEQUENCE);
 		} catch(Exception e) {}
 		boolean temporal = jsonObject.getBoolean(TEMPORAL);
+		boolean temporalAnnual = jsonObject.has(TEMPORAL_ANNUAL) && jsonObject.getBoolean(TEMPORAL_ANNUAL);


Unit tests are failing since the TEMPORAL_ANNUAL parameter is not found in the jsonObject. I'd suggest wrapping it in a try/catch as was done for TEMPORAL_SEQUENCE (see a few lines above it). You'll also want to make sure the FeatureExtraction .jar file is recreated once you've made those changes.

Anthony, it is rather a workaround not to adapt the existing tests

For some reason
boolean temporal = jsonObject.getBoolean(TEMPORAL);
doesn't fail

Catching an exception and not having any log entry might be dangerous in the future while troubleshooting

Referencing SqlRender 1.19.1 Targeting the 3.8.0 release

… element

alex-odysseus · 2025-02-24T19:43:01Z

Anthony, could you please check why some of the tests are still failing, for example test-GetCohortBasedCovariates? @anthonysena

Or test-FeatureExtractionInternal onLoad is expected to be silent

Is it possible that a version of SqlRender has an impact?

… tests will be passing after (it could be a great catch!)

alex-odysseus · 2025-02-24T20:41:50Z

Anthony, Finally, I found a bug, thanks to tests! @anthonysena

So only one is failing, please help

── Failure ('test-FeatureExtractionInternal.R:6:3'): Test .onLoad() ────────────
FeatureExtraction:::.onLoad(libname = "FeatureExtraction", pkgname = "FeatureExtraction") produced warnings.

anthonysena · 2025-02-28T14:19:17Z

Nicely done @alex-odysseus! So for the last error, this is due to the jarChecksum.txt being out-of-date. Could you update it using the following code: https://github.com/OHDSI/FeatureExtraction/blob/main/extras/PackageMaintenance.R#L87-L88 and then commit the updated inst/csv/jarChecksum.txt?

alex-odysseus · 2025-03-04T21:58:25Z

Anthony and Chris, I am getting a strange error, when I use a simple Java program to generate a checksum


import org.ohdsi.featureExtraction.JarChecksum;

public class JarChecksumGenerator {

	public static void main(String[] args) {
		System.out.println(JarChecksum.computeJarChecksum());

	}

}

java.io.FileNotFoundException: <my-source-folder-here>\FeatureExtraction\target\classes (Access is denied)
	at java.base/java.io.FileInputStream.open0(Native Method)
	at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:152)
	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:106)
	at org.ohdsi.featureExtraction.JarChecksum.computeJarChecksum(JarChecksum.java:29)
	at JarChecksumGenerator.main(JarChecksumGenerator.java:6)

Could you, please, try to generate it on your end? @anthonysena @chrisknoll

oleg-odysseus · 2025-03-17T17:36:44Z

Hi @anthonysena @chrisknoll , could you please take a look (I`ve generated the checksum for the jar file) and merge if windows/ubuntu images check is sufficient (mac failed due to a memory limit issue) and if you have no other objections. Thank you.

anthonysena · 2025-03-18T14:10:06Z

@oleg-odysseus this looks good and the Mac error has been noted in #294 so I think we're set to merge this. Let me just tag @ginberg for awareness and we'll work to get this into a release.

wivern and others added 5 commits February 21, 2024 19:09

ATL-10 Robust characterization analysis module (Table and Listings)

53c95f9

Fixes characterization import

a42620d

Enables tempoeral CC results

a3fccea

Aligning with the version of SqlRender (1.16.1) used in WebAPI

5766fb2

anthonysena self-assigned this Sep 9, 2024

anthonysena requested changes Sep 9, 2024

View reviewed changes

anthonysena mentioned this pull request Dec 17, 2024

Temporal annual distribution functionality and temporal FeatureExtraction capability enablement OHDSI/Atlas#2950

Merged

Merge remote-tracking branch 'remotes/upstream/develop' into temporal…

2c18bde

…-annual-distribution # Conflicts: # java/org/ohdsi/featureExtraction/FeatureExtraction.java # pom.xml

anthonysena requested changes Feb 18, 2025

View reviewed changes

alex-odysseus added 2 commits February 24, 2025 19:55

Fixing failing tests

cb3ab00

Referencing SqlRender 1.19.1 Targeting the 3.8.0 release

Fixing test-DetailedCovariateSettings to reflect the "temporalAnnual"…

8060b1a

… element

alex-odysseus added 2 commits February 24, 2025 21:01

Pertaining the order of fields as it was before refactoring to see if…

d1867f1

… tests will be passing after (it could be a great catch!)

Committing the JAR after the previous fix

7cff441

oleg-odysseus added 2 commits March 14, 2025 20:04

Rebuilt jar with JDK 1.8 and updated the jar checksum from R

f46d980

Fixed cache version in the pipeline

5a809fa

ginberg added this to the v3.8.0 milestone Mar 19, 2025

ginberg merged commit 3f0b2f1 into OHDSI:develop Mar 19, 2025
3 of 4 checks passed

Conversation

alex-odysseus commented Aug 28, 2024

Uh oh!

alex-odysseus commented Aug 28, 2024

Uh oh!

anthonysena left a comment

Choose a reason for hiding this comment

Uh oh!

anthonysena Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-odysseus Feb 24, 2025

Choose a reason for hiding this comment

Uh oh!

anthonysena commented Sep 10, 2024

Uh oh!

chrisknoll commented Feb 11, 2025

Uh oh!

anthonysena commented Feb 11, 2025

Uh oh!

anthonysena Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

alex-odysseus Feb 24, 2025

Choose a reason for hiding this comment

Uh oh!

alex-odysseus commented Feb 24, 2025

Uh oh!

alex-odysseus commented Feb 24, 2025

Uh oh!

anthonysena commented Feb 28, 2025

Uh oh!

alex-odysseus commented Mar 4, 2025

Uh oh!

oleg-odysseus commented Mar 17, 2025

Uh oh!

anthonysena commented Mar 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

anthonysena Sep 9, 2024 •

edited

Loading