OHDSI / Strategus

[Under development] An R packages for coordinating and executing analytics using HADES modules
https://ohdsi.github.io/Strategus/
7 stars 11 forks source link

Sync with ES Module v0.6.2 #158

Closed anthonysena closed 1 month ago

anthonysena commented 1 month ago

Migrate changes from https://github.com/OHDSI/EvidenceSynthesisModule/compare/v0.6.1...v0.6.2

anthonysena commented 1 month ago

@schuemie - I need some help on this PR. For reference, I've ported over the code from the ESModule repo to simulate the data and have stored that in a SQLite DB in the Strategus repo under inst/testdata/esmodule/results.sqlite. When I attempt to run the ES Module tests, I'm getting a failure which I've isolated to the case when we are running the bayesianMetaAnalysisSccs. The code here: https://github.com/OHDSI/Strategus/blob/9e80a3d9ac72848a7d1f5ff103f236cde8c740d6/R/Module-EvidenceSynthesis.R#L790-L809 The llApproximations contains columns outcomeId.x and outcomeId.y which usually indicates a problem in the join condition. I'm not sure what may be causing this - it could be a problem in the simulated data or an actual bug.

schuemie commented 1 month ago

@anthonysena : Right now I'm getting ERROR: object 'jobContext' not found because jobContext does not exist in this context. I could fix that, but I'm sure you have a preferred way to do that.

anthonysena commented 1 month ago

@anthonysena : Right now I'm getting ERROR: object 'jobContext' not found because jobContext does not exist in this context. I could fix that, but I'm sure you have a preferred way to do that.

Sorry for that @schuemie - this should now be fixed in https://github.com/OHDSI/Strategus/pull/158/commits/3c7fcf4d036b564f36e28280ee11301f6ed311b2

schuemie commented 1 month ago

Weird, the error is in my original simulation code, so I'm not sure why we were passing unit tests earlier. The problem is that the simulated sccs_likelihood_profile table has an outcome_id column, as generated here, but according to the specs it should not have that column (the outcome ID is in the exposures_outcome_set table).

We could just delete this column from the simulation code and it should work. However, even though the code in Strategus references extras/ESModule-SimulationFunctions.R here, this file currently does not exist in Strategus. Is it just a verbatim copy of the file in the EvidenceSynthesisModule repo?

anthonysena commented 1 month ago

Apologies again @schuemie I've added extras/ESModule-SimulationFunctions.R to this branch and as you will see it is a verbatim copy of what was in the ESModule repo (I only changed the file name at this point). It is possible that the SQLite DB in the ESModule repo is out-of-sync with the simulation code and it is missing the outcome_id column in the sccs_likelihood_profile table.

I will remove the outcome_id from sccs_likelihood_profile and retest this now.

anthonysena commented 1 month ago

Thanks @schuemie - I think that change fixed it. Tests now pass for me locally. I did want to note that when I regenerated the simulated data, I did encounter an error:

Simulating outcome 2
Inserting data took 0.0208 secs
Inserting data took 0.0167 secs
Inserting data took 0.0188 secs
Converting person data to SCCS intervals. This might take a while.
  |=================================================================================================================================| 100%
Generating SCCS interval data took 1.35 secs
Fitting SCCS model
Using prior: None
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Fitting the model took 0.388 secs
Model fitting status is: OK
Converting person data to SCCS intervals. This might take a while.

<-- removed to save space --> 

|=================================================================================================================================| 100%
Generating SCCS interval data took 1.26 secs
Fitting SCCS model
Using prior: None
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Using 1 thread(s)
Fitting the model took 0.347 secs
Model fitting status is: OK
Converting person data to SCCS intervals. This might take a while.
  |====================================================================================================                             |  77%Generating SCCS interval data took 3.27 secs
Fitting SCCS model
Fitting the model took 0.179 secs
Model fitting status is: Could not estimate because there was no data
Error in UseMethod("rename") : 
  no applicable method for 'rename' applied to an object of class "NULL"
In addition: There were 50 or more warnings (use warnings() to see the first 50)

I'm not sure why there is no data for this particular outcome but since the code is aiming to simulate 26 outcomes and this failed on the 2nd outcome I wanted to note it.

schuemie commented 1 month ago

Thanks! Yes, the simulation code doesn't always generate events (it's random). Run it again and it may have events for all simulated outcomes, but this is fine too.