search

cdisc-org / DDF-RA

This is the repository for all code and documentation for the DDF-RA project.
MIT License
18 stars 1 forks source link

Not possible to reference a "global" eligibility criterion #388

Closed ASL-rmarshall closed 4 months ago

ASL-rmarshall commented 5 months ago

According to the IG, the context relationship between EligibilityCriterion and either StudyVersion or StudyDesign is used to indicate whether the criterion applies to the whole study or or a specific study design:

The context of each eligibility criterion should be indicated either by a reference to the whole study (StudyVersion class) or to a particular study design (StudyDesign).

However, according to the API specification, each eligibility criterion is defined within the context of a specific StudyDesign - either within a StudyDesignPopulation (which is defined as the population for a StudyDesign) or within a StudyCohort (which is defined as one of the cohorts for a StudyDesignPopulation, which is defined as the population for a StudyDesign). As eligibility criteria are included in populations/cohorts by definition (not by reference to an id value), there doesn't appear to be a way to define a "global" criterion (i.e., one with a context relationship to the StudyVersion) once and to reference it in multiple study designs. It looks like each "global" criterion would have to have multiple duplicate definitions - one for each of the study designs where the criterion applies.

BSnoeijerCD commented 5 months ago

@dih-cdisc : how do you want to handle this one? The criteria are nested in populations now. To allow reuse within multiple study designs and across cohorts/populations I believe the best solution is to nest all criteria in studyVersion and make the relationship from studyPopulation to them a cross reference in the api. This might require only a change in API and potentially in the IG and/or rules.

Population_suggested.png
dih-cdisc commented 5 months ago

@BSnoeijerCD Yes, agree. Put the collection at the StudyVersion level and all other instances can cross reference to the collection. Will need a CT change as there is a new relationship from StudyVersion ("the set of criteria for all study designs within the study version")

ASL-rmarshall commented 5 months ago

@dih-cdisc @BSnoeijerCD I agree that it makes sense to move the definitions of eligibility criteria to be within StudyVersion as described. Am I right to presume that:

  1. The relationship between StudyVersion and EligibilityCriterion will (also) be labelled criteria?
  2. In the API spec, the criteria property for both StudyDesignPopulation and StudyCohort will be renamed as criterionIds and "re-typed" as an array of any of string or null?
  3. We'll need a rule that says that each defined eligibility criterion must be referenced by at least one StudyDesignPopulation or StudyCohort?

I'm just wondering what will happen to the context relationships that kicked off this discussion in the first place. Will they still be needed? I guess each criterion will be referenced either by the population/cohorts in all study designs (a "whole study" criterion) or only by the population/cohorts in a subset of the study designs (a "study-design-specific" criterion). Is it still necessary to flag each criterion specifically as whole study/study-design-specific by also creating either

...or is "whole study"/"study-design-specific" an all or nothing property - i.e., if a criterion is used by more than one study design, it should be flagged as "whole study" with a context relationship to StudyVersion (even if it is not used by all study designs)? And a "study-design-specific" criterion is one that's only used in a single study design?

dih-cdisc commented 5 months ago

@dih-cdisc @BSnoeijerCD I agree that it makes sense to move the definitions of eligibility criteria to be within StudyVersion as described. Am I right to presume that:

  1. The relationship between StudyVersion and EligibilityCriterion will (also) be labelled criteria?
  2. In the API spec, the criteria property for both StudyDesignPopulation and StudyCohort will be renamed as criterionIds and "re-typed" as an array of any of string or null?
  3. We'll need a rule that says that each defined eligibility criterion must be referenced by at least one StudyDesignPopulation or StudyCohort?

I'm just wondering what will happen to the context relationships that kicked off this discussion in the first place. Will they still be needed? I guess each criterion will be referenced either by the population/cohorts in all study designs (a "whole study" criterion) or only by the population/cohorts in a subset of the study designs (a "study-design-specific" criterion). Is it still necessary to flag each criterion specifically as whole study/study-design-specific by also creating either

  • a single context relationship to StudyVersion or
  • a context relationship to any StudyDesign that contains a population/cohort that uses the criterion (note that the 0..1 cardinality on the context relationship to StudyDesign wouldn't allow more than one study design to be referenced, which doesn't seem right)?

...or is "whole study"/"study-design-specific" an all or nothing property - i.e., if a criterion is used by more than one study design, it should be flagged as "whole study" with a context relationship to StudyVersion (even if it is not used by all study designs)? And a "study-design-specific" criterion is one that's only used in a single study design?

Not totally convinced, going to have a ponder on this one. Need to draw a picture

BSnoeijerCD commented 5 months ago

@dih-cdisc @EMuhlbradt @czwickl: I added the criteria relationship to the StudyVersion class. See below. @ASL-rmarshall I defined CHk0173 for this one in v4.0.

Population_annotated1.png
BSnoeijerCD commented 5 months ago

Uploaded UML and views to github and updated IG as well.

dih-cdisc commented 5 months ago

I think we can dispose of the "context" relationships from EligibilityCritieria

dih-cdisc commented 5 months ago

API updated

ASL-rmarshall commented 5 months ago

@BSnoeijerCD I've reviewed CHK0173 and suggested a tweak to the wording - I interpreted this check as making sure that there are no unused eligibility criteria defined in a study version (with it not mattering if each criterion is used in population(s) and/or cohort(s)). However, I'm wondering if we (also) need a check to make sure that, within the same study design, each eligibility criterion is used either by the population or by 1 or more of the cohorts (not both population and cohort(s)).

If we're dispensing with the context relationships for EligibilityCriterion then CHK0074 should be changed to N for USDM 4.0.

BSnoeijerCD commented 5 months ago

@dih-cdisc: removed context and UML updated and uploaded to github. @EMuhlbradt and @czwickl : that means you can remove the context attribute in CT from:

@ASL-rmarshall: we can also decide not to delete CHK0074 as it is difficult to interprete the context for the criteria and we are improving it in v4.0 anyway.

Regarding CHK0173: you are right that this is the check that each defined eligibility criterium is used. Can you add your suggested updated text? I will add another v4.0 check to check that the same criterium cannot be used by both the studyDesignPopulation and a StudyCohort within the same study design.

BSnoeijerCD commented 5 months ago

@EMuhlbradt and @czwickl : Can you take care of removing the context attribute in CT from PopulationDefinition, StudyDesignPopulation and StudyCohort as indicated above? I think it is not yet in your latest version of changes.

EMuhlbradt commented 5 months ago

@EMuhlbradt and @czwickl : Can you take care of removing the context attribute in CT from PopulationDefinition, StudyDesignPopulation and StudyCohort as indicated above? I think it is not yet in your latest version of changes.

Berber, these attributes are not present in the latest CT deliverables. They must have been taken out previously? The 'context' relationship is only present in Condition and EligibilityCriterion classes currently.

BSnoeijerCD commented 5 months ago

@EMuhlbradt and @czwickl : Can you take care of removing the context attribute in CT from PopulationDefinition, StudyDesignPopulation and StudyCohort as indicated above? I think it is not yet in your latest version of changes.

Berber, these attributes are not present in the latest CT deliverables. They must have been taken out previously? The 'context' relationship is only present in Condition and EligibilityCriterion classes currently.

Apologies, My error: Can you remove context from the EligibilityCriterion class?

EMuhlbradt commented 5 months ago

@BSnoeijerCD done; see file (sprint 3 changes tab): DDF.Terminology_Sprint.3.changes_2024-06-10_em.xlsx

BSnoeijerCD commented 5 months ago

@EMuhlbradt @czwickl - thank you for the update @dih-cdisc - IG text changed accordingly @ASL-rmarshall - I will set CHK0074 to deleted - it is no use creating this rule because

ASL-rmarshall commented 5 months ago

@dih-cdisc @BSnoeijerCD Quick question before we release this with v3.2: is it necessary to represent the criteria relationship between StudyVersion and EligibilityCriterion in the UML? It seems quite like a "collection" relationship so you could argue that it should only be represented in the API spec.

dih-cdisc commented 5 months ago

@dih-cdisc @BSnoeijerCD Quick question before we release this with v3.2: is it necessary to represent the criteria relationship between StudyVersion and EligibilityCriterion in the UML? It seems quite like a "collection" relationship so you could argue that it should only be represented in the API spec.

It would do no harm to remove it from the UML, it is more of a collection. The "natural" parent of EligibilityCriterion is PopulationDefinition but so as not to create duplicate content (i.e. same criterion referenced from several places) we introduce the criteria from StudyVersion to ensure only included once.

@BSnoeijerCD if you want to change it do it for v3.2.0 (i.e. now)

BSnoeijerCD commented 5 months ago

@dih-cdisc @BSnoeijerCD Quick question before we release this with v3.2: is it necessary to represent the criteria relationship between StudyVersion and EligibilityCriterion in the UML? It seems quite like a "collection" relationship so you could argue that it should only be represented in the API spec.

It would do no harm to remove it from the UML, it is more of a collection. The "natural" parent of EligibilityCriterion is PopulationDefinition but so as not to create duplicate content (i.e. same criterion referenced from several places) we introduce the criteria from StudyVersion to ensure only included once.

@BSnoeijerCD if you want to change it do it for v3.2.0 (i.e. now)

Yes. Will do. That means that the criteria relationship also needs to be removed from CT class StudyVersion: @EMuhlbradt and @czwickl