cdisc-org / DDF-RA

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

Add BC Capability #47

Closed dih-cdisc closed 1 year ago

dih-cdisc commented 1 year ago

See ticket DDF-RA #18

dih-cdisc commented 1 year ago

BC Collections / Recursion / Groupings are all referring to the same concept on being able to group BCs into various levels.

e.g.,

When building a study a user may select from any level, the information being taken from a Library (e.g. CDISC Library) and included into the USDM (Study)

cupkes commented 1 year ago

I have been going back and forth over the semantics of this all night. I'll create an actual class diagram shortly, but one thing I have decided on is that there will likely be two classes: one that plays a piece in the ontology and one that associates BCs with the ontology. The mechanics are straightforward. The semantics (class names, etc..) I've been flip-flopping on. I'll try and just make a command decision here shortly.

cupkes commented 1 year ago

BiomedicalConcepts

cupkes commented 1 year ago

So, at first I had two different types of classes, one the parent class Category and a sub class CategoryLeaf. This polymorphism would just confuse too many. So, the rules for when members can be added is now not the job of the SDR class. That logic would exist in the ontology or taxonomy builder class that generates the doubly-linked list of of categories.

cupkes commented 1 year ago

As for the BC itself, not sure of what kind of Code or AliasCode properties it might have. Just have a label (non-conformant) at the moment.

cupkes commented 1 year ago

Also note, this design does not restrict the ontology to single inheritance as any category can have multiple parents..

EMuhlbradt commented 1 year ago

CT spreadsheet containing Sprint 7 items and questions in yellow in column M for @daveih to review. DDF Terminology_Sprint 7 changes_2023-01-20.xlsx

cupkes commented 1 year ago

BiomedicalConcept

dih-cdisc commented 1 year ago

Responses to @EMuhlbradt @czwickl DDF.Terminology_Sprint.7.changes_2023-01-20.xlsx

cupkes commented 1 year ago

BiomedicalConcept

cupkes commented 1 year ago

Let's discuss.

cupkes commented 1 year ago

BiomedicalConcept

cupkes commented 1 year ago

One more time: BiomedicalConcept

EMuhlbradt commented 1 year ago

Question from Erin/Craig: What do you mean by 'bcPropertyDatatype'? Do you actually mean the datatype of the property itself or the datatype of the valid responses for the stated property? Is the attribute better named as 'Biomedical Concept Property Response Data Type'

The model seems to imply that bcPropertyDatatype is a string based on the latest model diagram?! So why do we need this info? Shouldn't the datatype be on the response code instead?

EMuhlbradt commented 1 year ago

Question from Craig/Erin: What do the modelers mean by a biomedical concept surrogate? Like A1C level being a surrogate for diabetes diagnosis? Or something else entirely?

EMuhlbradt commented 1 year ago

Please see attached draft CT for sprint 7 and biomedical concept modeling: DDF.Terminology_Sprint.7.changes_2023-01-30.xlsx

ggasg commented 1 year ago

See below for an updated diagram, including the impact of adding BC to the current model

Screen Shot 2023-01-30 at 21 56 18
dih-cdisc commented 1 year ago

Question from Craig/Erin: What do the modelers mean by a biomedical concept surrogate? Like A1C level being a surrogate for diabetes diagnosis? Or something else entirely?

It is for when we know we need a BC but dont have a BC definition available, we can fill in a placeholder definition, maybe point at a LOINC or Snomed code for an equivalent definition

dih-cdisc commented 1 year ago

Question from Erin/Craig: What do you mean by 'bcPropertyDatatype'? Do you actually mean the datatype of the property itself or the datatype of the valid responses for the stated property? Is the attribute better named as 'Biomedical Concept Property Response Data Type'

The model seems to imply that bcPropertyDatatype is a string based on the latest model diagram?! So why do we need this info? Shouldn't the datatype be on the response code instead?

It is the response datatype but the name gets too long. The model names are implementation "unfriendly" as it is.

EMuhlbradt commented 1 year ago

Comment from Erin/Craig: There appears to be some mis-alignment between Dave's latest Ball and Stick diagram and the latest model pict from Gaston. Which is the source of truth?

  1. In the ResponseCode class, Dave's diagram has 'code' as an attribute only while Gaston's model has 'code' as an attribute AND a relationship.
  2. In the BiomedicalConceptCategory class, Dave's diagram has one additional attribute 'bcReference' and two additional relationships 'bcCategoryParents' and 'bcCategoryChildren'. Gaston's model does not have these.
  3. In the BiomedicalConcept class, Dave's diagram has an attribute 'bcConceptId' while Gaston's model has 'biomedicalConceptId' as the attribute. We think these are the same.
  4. In the BiomedicalConceptProperty class, Dave's diagram has an attribute 'bcPropertyConceptId' while Gaston's model shows an attribute of 'bcPropertyId'. Please check inconsistency. Also please note that Gaston's model shows 'bcPropertyResponseCodes' as an attribute and a relationship while Dave's diagram only shows 'bcPropertyResponseCodes' as a relationship. What's correct?
  5. None of the new attributes and relationships for the Activity class are reflected in Dave's diagram.
dih-cdisc commented 1 year ago

Response to above for @EMuhlbradt and @czwickl

  1. I think you are referring to the filled in diamond and arrow line, if so, all OK.
  2. Removed 'bcReference' from DIH diagram. The relationships on DIH are the List attributes, it is a drawing style. Where in the UML you see an attribute that has a type of another class it is a "relationship" but in the UML does not get drawn as such. So OK. Follow the UML.
  3. No. In Gaston's diagram this is the id of the BC. In mine I don't show the id fields for any class. This bcConceptId (bad choice of name) is the CT reference for the definition of the BC, so the C Code for "Systolic Blood Pressure". Use an alias code in case we want to refer to other terminologies
  4. For the concept Id same answer as above. The relationship thing is OK, style issue as above
  5. Amended the names in the DIH picture of the three relationships relating to the List attributes. Should now align
dih-cdisc commented 1 year ago

Given @EMuhlbradt's checking @ggasg, couple of tweaks needed

  1. Activity class, studyDataCollection attribute can be deleted (replaced by the surrogate class)
  2. Activity class 'biomedicalConcept' attribute name -> 'biomedicalConcepts' ... add an 's'
  3. In BiomedicalConcept we need an id attribute (biomedicalConceptId) which is there. But we also need a ref to an aliasCode that holds the CT definition of what the BC is called. I called it bcConceptId which is a really bad name, so we need to add the attribute and think of a better name
  4. As per 3, same issue in BiomedicalConceptProperty
dih-cdisc commented 1 year ago

New DIH sketch for @EMuhlbradt @czwickl @ggasg Note the blue comments down bottom left of diagram

2023 01 31 DDF Model Plus BCs

ggasg commented 1 year ago

@dih-cdisc @EMuhlbradt @czwickl Thank you very much for the feedback. Model has been updated to reflect the corrections. Also, at the risk of proposing something off the mark:

  1. BiomedicalConcept: bcConceptDefinition (AliasCode)
  2. BiomedicalConceptProperty: bcPropertyDefinition (AliasCode)

Thoughts? See below for updated diagram.

Screen Shot 2023-01-31 at 14 11 29
EMuhlbradt commented 1 year ago

Regarding BiomedicalConcept: bcConceptDefinition (AliasCode) and BiomedicalConceptProperty: bcPropertyDefinition (AliasCode): I'm worried that we are using the word 'definition' and 'name' interchangeably and I'm not sure we want to do that. Based on @dih-cdisc comment: 'holds the CT definition of what the BC is called'....so does it hold a name or definition? If a name, is it the standard or preferred name given by the BC source?

dih-cdisc commented 1 year ago

The information we get from the CDISC API for a BC is this.

For the BC itself

Screenshot 2023-02-01 at 06 27 41

For a property

Screenshot 2023-02-01 at 06 27 50

So, I assume, the C code "conceptId" is the definition

dih-cdisc commented 1 year ago

@dih-cdisc @EMuhlbradt @czwickl Thank you very much for the feedback. Model has been updated to reflect the corrections. Also, at the risk of proposing something off the mark:

  1. BiomedicalConcept: bcConceptDefinition (AliasCode)
  2. BiomedicalConceptProperty: bcPropertyDefinition (AliasCode)

Thoughts? See below for updated diagram. Screen Shot 2023-01-31 at 14 11 29

@ggasg @czwickl @EMuhlbradt I am good with the attribute names suggested as long as Craig/Erin happy.

dih-cdisc commented 1 year ago

Image sent via email on Wednesday 1st Feb 2023 02 01 DDF Model Plus BCs

dih-cdisc commented 1 year ago

Updated version of sketch

2023 02 03 DDF Model Sprint 7