DefiningContextConstraint --> LocationConstraint, AlleleConstraint

[DanielPuthawala]

On the 10/15/2024 Cat-VRS call, a question was raised as to whether the DefiningContextCosntraint should be broken up due to the fact that it contains both location and sequence state information. The suggestion has been to instead have two constriants, the LocationConstraint, and aseparately an AlleleConstraint, to capture the location and state properties of catVars separately. This discussion will be held here to prevent the main constraint model discussion thread from getting too long.

[DanielPuthawala]

Here's an initial mock-up of how we could split up the DefiningContextConstraint (and the GeneContextConstraint that was sort of floating around in the CNV/Function variant space) into the two new independent LocationConstraint and AlleleConstraint. We will discuss this during our next meeting, and as always, I'd also love to discuss it here!

[larrybabb]

@DanielPuthawala re: AlleleConstraint to replace DefiningContextConstraint proposal above.

1. Dropping the qualifier Defining from the AlleleConstraint changes the definition of the constraint to be something that is at the root of the containing CategoricalVariation to being more of a basic constraint. Other basic Constraints (like CopyCountConstraint) act like filters as far as I've understood and thus set limitations on what members characteristics match. An AlleleConstraint to me would mean that all members have to match the provided allele which is different than a "defining" or "originating" allele. We discussed this terminology several years ago with the ClinGen Allele Registry developers and landed on either "defining" or "originating" as good qualifier terms to mean that all members are defined by or originate from this "defining" or "originating" allele. I think this is important to leave in the name of the Constraint. I suggest DefiningAlleleConstraint.

2. The allele property should simply be of type vrs:Allele. I don't think we would ever mix the SequenceExpression subclasses there or use them as allele representations. I also don't think we should add the CisPhasedBlock at this time. The idea of creating a CanonicalCisPhasedBlock is definitely needed so that we can represent PGx star alleles going forward, but I think we should separate that concept from the CanonicalAllele concept. I would be supportive of adding a DefiningCisPhasedBlockConstraint as a draft of how we may begin representing canonical haplotypes and star alleles.

3. I don't understand what the sequenceCollection attribute provides and how that impacts the overall constraint and thus the containing CategoricalVariant's members. Please clarify or consider dropping until we can discuss as a group.

4. How will we support the requirement of having a DefiningAlleleConstraint that is limited to a genomic, transcript or protein sequence? I don't think we want to use the same Constraint for both ProteinExpression and CanonicalAllele.

@ahwagner I would also like your $.02 on the above ☝️ as well as anyone else that has solutions to these concerns.

[ahwagner]

1. @DanielPuthawala and I discussed this proposal and your comments, and agree that defining is important to maintain for the reasons specified here.
2. We agree and will adjust this to just support Allele classes for DefiningAlleleConstraint, and we agree about the potential for a DefiningCisPhasedBlockConstraint but will return to this later as motivated by use cases.
3. We agree and will drop the sequenceCollections attribute
4. We will create a MolecularContextConstraint that is based on an enum of context types to be resolved in Seqref updates vrs#573 and SequenceReference molecule type vrs#567, and then add this constraint to the "recipe" for CanonicalAllele. We also clarified the use of "profile" vs. "recipe" across GKS products: a profile is a filtering of class properties to create a more constrained class, whereas a recipe is using multiple data class components together to create a certain type of expression using all of those components.

[larrybabb]

re: LocationConstraint to replace both GeneContextConstraint and DefiningContextConstraint

Assumption
I believe the primary purpose of this constraint is to provide a genomic, transcript or protein sequence location from which different types of alignment or translation can be made to other types and/or versions of sequence locations based on a predefined set of enumerated terms 'protein-translation, 'transcript-projection' and 'genome-liftover'. This constraint is more complex than a simple "filtering" type constraint since it both defines the "originating" location (or gene) and then describes which alignment or mapping strategies are allowed.

1. Should we refer to the primary attribute that contains the defining sequence location as origin or source or basis? And should we limit the type to SequenceLocation and Gene?

2. How will we clarify when the origin is limited to a 'protein', 'transcript' or 'genomic' sequence (same issue as AlleleConstraint above)?

3. If this constraint is representative of something that specifies an originating sequence location (or gene) then do we really need to specify the permitted mapping approaches? Is there a case when someone wouldn't use both liftover and projection for originating genomic sequence locations? and when someone uses an originating transcript sequence location I think there would be a defined assumption that it would include any genomic-projections as well as the protein-translations. Finally, if the originating sequence location was a protein sequence then one would reasonably define these alignment members as being the transcript-translation and any genomic-projections from those transcript sequence locations. This pattern of how originating or defining contexts (whether they be locations, genes, alleles or ?) is common enough IMO that we should simply bake them into the definitions of the DefiningLocationConstraint and DefiningAlleleConstraint.

4. I don't understand what the sequenceCollection attribute provides and how that impacts the overall constraint and thus the containing CategoricalVariant's members. Please clarify or consider dropping until we can discuss as a group.

[ahwagner]

We are heavily revising the above model and will not be combining Gene and Location constraints.

1. Since we will be keeping these separate, there will be no need to make the distinction you propose here; the definingLocationConstraint will only support Location
2. Same as above, we will have a separate constraint for this information
3. Yes, the operations are helpful, they allow us to broaden (or not) the notion of knowledge that is generalizable to other sequences (or not). IMO, we should keep the relations. E.g. gnomAD data is explicitly recalculated for different genome assemblies, not lifted over, and only applicable for those contexts for which it is reported
4. I agree and we're not really sure how this got back in, but we're dropping it.

[DanielPuthawala]

Ok, here's the updated draft mock-up based on the excellent discussion yesterday!

Summary of changes from previous mockup:

• Whereas the location corresponding to a gene cna be specified in the DefiningLocationConstraint, the GeneContextConstraint is being kept separately, for capturing references to genes as heritable units without making reference to their corresponding location on a given reference genome.. For example, the CatVar _BRAF_ Amplification is a kind of Copy-Number Gain vairant, and what is being gained is the copies of the BRAF gene, but without any presupposed information as to where the additional copies are in the genome.
• The LocationConstraint is now the DefiningLocationConstraint, and the Location property must be a vrs:Location.
• The AlleleConstraint is now the DefiningAlleleConstraint, and must consist of a vrs:Allele. Also, reference to sequenceCollections was removed. That ended up in there because I copied over a defunct sketch of the DefiningContextConstraint by accident.
• Added the MolecularContextConstraint, which will be used to represent the type of the vrs:Allele in the DefiningAlleleConstraint as being a genomic, transcript, or protein sequence, etc. This would be a new constraint.

Does this look more like what you were expecting @larrybabb?

[larrybabb]

This is a much better, thank you for addressing my concerns. I'm not 100% sure about the MolecularContextConstraint and how it is applied differently in the DefiningAlleleConstraint vs using relations in the DefiningLocationConstraint. My understanding is that whether you are lifting over, projecting and/or translating sequences between different molecule contexts you would do it in a very similar (if not exact) way whether it was a sequence location for an Allele or a SequenceLocation itself (they are the same thing). So why wouldn't we model these recipes in the same way with the only differnce being a Location on an Allele as the defining context source for the members?

[larrybabb]

I just went back and re-read @ahwagner responses above.

1. I think @ahwagner was stating that the new MolecularContextConstraint would be needed in recipes with either DefiningLocationConstraint or DefiningAlleleConstraint so that makes sense to me, but I'm not 100% sure what value it provides other than re-ifying the SequenceReference.moleculeType value in the DefiningXXXConstraints allele or location property. Does this mean that we will need to validate that the MolecularContextConstraint.enum always matches the DefiningAlleleContext.allele.location.sequenceReference.moleculeType value?

2. I also think that @ahwagner did not mean to have us drop the relations array attribute from DefiningLocationConstraint since we will need to indicate whether the members in this class are lifted-over, projected, or translated between molecule types. If this is intentionally dropped, I'd like to know how it one would use DefiningLocationConstraint in a manner similar to DefiningAlleleConstraint.

[ahwagner]

1. Yes, this new constraint has been added as a way of specifying that a DefiningAlleleContext.allele.location.sequenceReference.moleculeType is of a particular type. This is to help with specifying that a canonicalAllele, for example, is based on a chromosome reference context. As with all constraints, this would only need to be validated when this constraint is used.
2. I agree, relations was not supposed to be dropped

[DanielPuthawala]

Larry, I didn't drop relations. It's right there in the DefiningLocationConstraint. What are you talking about?