ohdsi / skeletoncohortcharacterization Goto Github PK
View Code? Open in Web Editor NEWLicense: Apache License 2.0
License: Apache License 2.0
In a case where the prevalence shows a count of 121 and a percentage of 3.82e-04, but should be 1.17e-03 because it is using the denominator as cohort episodes and not distinct people in cohort. Ie: prevalence should be distinct people with criteria / distinct people in cohort.
It is calculating distinct people / cohort episode count.
Cross posting this issue (OHDSI/WebAPI#1433) here. For CC to work in Spark SQL, the cc_results inserts would explicitly use all columns rather than a selection.
Referring to these lines in the prevalence criteria query, we see that the count of people found with the criteria is performed as a distinct person count (via the group by person_id
) but the deominator is calculated by select count(*) from cohort
.
The issue is that if there are multiple episodes per person, a criteria that matches against all people in the cohort will have a < 100% prevalence because distinct people < total records.
Solution: make the totals subquery count distinct person_id.
I'm finding a few places where the cohort table schema is being confused between the result schema and temp schema:
return getSqlQueriesToRun(createFeJsonObject(options, options.resultSchema + "." + cohortTable), cohortDefinitionId);
String cohortTable = Optional.ofNullable(strata).map(this::getStrataCohortTable).orElse(tempSchema + "." + this.cohortTable);
This might work if the resultsSchema is the same as the tempSchema, but in the case they are different, I believe this would break. However, looking elsewhere in the code, I see something strange when using createDefaultOptions
:
final CohortExpressionQueryBuilder.BuildExpressionQueryOptions options = new CohortExpressionQueryBuilder.BuildExpressionQueryOptions();
options.cdmSchema = cdmSchema;
// Target schema
options.resultSchema = tempSchema;
options.cohortId = id;
options.generateStats = false;
return options;
One element of confusion is re-suing the CohortExpressionQueryBuilder.BuildExpressionQueryOptions
: this object encapsulates options for cohort definition generation, but the re-use of it in characterization seems strange here. But the main point of confusion is assigning the options.resultSchema = tempSchema
. Again, this works if resultSchema == tempSchema
, but doing this sort of overwrite here is very confusing to me.
For background: the purpose of defining the cohortTable
in the other analysis methods (or even as the targetTable
in cohort definitions) was that we don't assume anything about the location of the cohort table: it could be in a completely different schema in the database (something other than result
or temp
schema). It purpose was that the user would have to specify the fully-qualified table name of the cohort table. Therefore, I believe the solution to the confusion here is to make the callers of the CCQueryBuilder()
constructor pass the fully qualified name of the cohort table, and don't attempt to do any tricks with concatenating 'tempSchema' or 'resultsSchema': the cohort table is simply the cohort table (including schema).
It looks like when dealing with strata, a strata-cohort table is created:
return String.format("%s.sc_%s_%d", tempSchema, sessionId, strata.getId());
Which, in this case: the cohort table generated is the fully qualified name, so it does feel like it would be consistent to pass in the 'cohortTable' as the fully qualified name instead of trying to figure out which schema the cohort table belongs to.
I found two Cohort Characterization SQLs that do not use @vocabulary_database_schema
, for example in distributionRetrieving.sql line 29:
left join @cdm_database_schema.concept c on c.concept_id = fr.concept_id;
If rewritten as:
left join @vocabulary_database_schema.concept c on c.concept_id = fr.concept_id;
it could fetch concepts from the vocabulary daimon.
The same goes with prevalenceRetrieving.sql
.
PS: I was trying to fix this on current WebAPI release, but maybe it's better fix here and wait for 2.8?
At the moment, v1.0.0 of this project is deployed to the OHDSI nexus repo but no tag exists for the v1.0.0 release of this project. The v1.0.0 release was made at this commit point: 7cdb50f.
Additionally, the R DESCRIPTION file is out-of-sync with the versioning scheme defined in the pom.xml: https://github.com/OHDSI/SkeletonCohortCharacterization/blob/master/DESCRIPTION.
Suggested fixes:
While trying to run through the code in SkeletonCohortCharacterization as a R package, I discovered that all the aggregate functions are specified in the design JSON, but those specifications are not part of the SkeletonCohortCharacterization. Instead, these aggregate types are defined as entities in WebAPI, and then handled by passing in the raw definitions over to SkeletonCC.
I think this is wrong: SkeletonCC should define all the aggregate functions that are supported, and not allow external parties to submit their own expressions (isn't that a SQL injection problem?!?). The SkeletonCohortChacterization should expose a function to read meta about the available aggregate types for WebAPI to consume/render...but webAPI shouldn't be the place to define the characterization aggregate functions...It makes using SkeletonCC hard to use as a stand-alone library.
Cross posted from OHDSI/Atlas#2420
Expected behavior
When using subgroups in cohort characterization, the denominators used for percents should be for each subgroup.
Actual behavior
When using cohort event date ranges as subgroups (Jan-Apr, May-July, etc), the percents are using the total cohort as the denominator rather than the subgroup patients.
Steps to reproduce behavior
Create a cohort characterization, add some features, and add some subgroups.
PROBLEM ATTEMPTING TO SOLVE
Many OHDSI projects have aged libraries.
vulnerabilities exists in older libraries
Also many projects used different versions of same libraries
Projects like WebApi that depend on several libraries have to create very messy workarounds to exclude library versions that create conflicts
Duplicated Snippets in across projects
PROPOSED SOLUTION
update versions of maven tooling and used libraries
introduce spring-boot-starter-parent as a parent project where applicable
Ideally we would introduce a ohdsi-maven-starter-parent which can be shared across projects. This project would contain maven tooling, jacaco and others like repositories.
The skeleton now includes FeatureExtraction and SqlRender both as R packages (see DESCRIPTION) and as Java libraries (see inst/java). Why can't this skeleton be written as pure R?
The resulting R package is very big, and I don't think it is a good idea to have such big skeletons in Hydra.
I am confused what the function of the package is.
Can I provide a cohort id and have this package describe it for me (like Herakles?)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.