Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selection label examples #5

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Selection label examples #5

wants to merge 8 commits into from

Conversation

Simard302
Copy link
Collaborator

Overview

This PR creates 2 new examples for creating labels based on selections.

Changes made

  • Created selection example "add_label_to_selection_whole"
    • This examples shows you how to add a label to a grouping of atoms based on a selection
  • Created selection example "add_labels_to_selection_atoms"
    • This example shows you how to add a label to singular atoms (in this cased Alpha Carbon) based on a selection

How to test

Ensure the selection is visible

Specific review requests

For each of the examples:

  • Are there enough comments to sufficiently describe the process?
  • Are the comments accurately describing the process?
  • Are there any anti-patterns in the examples?
  • Can the examples be more efficient?

@Simard302 Simard302 requested a review from papillot June 10, 2024 14:05
Copy link
Collaborator

@papillot papillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ended-up being quite complex, not because of the labels per se, but more because of the way StructureSelection are laid out from the Query and Loci are defined from units.

I think there is probably an antipattern with the add_labels_to_selection_atoms example because we are trying to create on representation for each label and that makes 140+ representations in one go. There is a noticeable pause in the rendering.
It would be better to use a way to create the label that's more efficient and would serve as "The Way (™️)"

The add_label_to_selection_whole is working kind of by chance: the Query.generator.chains construct creates a grouping of the matches by chain id. Fortunately, this is the same mapping as when we use the label_asym_id property.
But this is not the case if we'd use the auth_asym_id property. In this case, we'd have 3-4 labels per "chain". Probably the best, in that case, is to use a groupBy callback with a Query.generators.atoms (not Query.generators.chains because this overwrites whatever groupBy property there is).
I guess such an example would be actually more generic.

@@ -31,7 +31,7 @@ async function init() {
// Create a selection for chains A and B

// A query defines the logic to select elements based on a predicate.
// The predicate is executed from a generator which updates the current element (here, an atom)
// The predicate is executed from a generator which updates the current element (here, a chain)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it's an atom that's returned as the argument for the predicate, but that we are iterating only through the 1st one in each chain

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the code in structure/query/queries/generators the chainTest is executed with the element corresponding to the chainSegment.start of each chain segment.

Copy link
Collaborator

@papillot papillot Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:
The predicate is executed from a generator that iterates over the structure hierarchy (chains, then residues, then atoms). At each iteration, the query context that is passed, is updated.
The chainTest predicate is executed once per chain. The element property represents the atom at the start of the current chain.
If the predicates return true, the entire chain segment is added to the selection.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

const ctx = new QueryContext(struct);
const selection = query(ctx);

// A StructureSelection is a Singleton if each iteration only adds a single element (atom)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is a Singleton" is not correct. Notice that the type is StructureSelection.Singletons (plural)
Suggestion:
// A StructureSelection is made of Singletons if each iteration only...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 29 to 30
// The predicate is executed from a generator which updates the current element (here, a chain and an atom)
// in the query context, at each iteration. The predicate returns true if the element should be selected.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The predicate is executed from a generator that iterates over the structure hierarchy (chains, then residues, then atoms) and groups matches by residue. At each iteration, the query context that is passed, is updated.
The chainTest predicate is executed once per chain. The element property represents the atom at the start of the current chain.
The atomTest predicate is executed for each atom in the structure that passes the chainTest.
If both predicates return true, the atom is added to the selection.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 32 to 33
atomTest: ctx => StructureProperties.atom.auth_atom_id(ctx.element) === 'CA',
chainTest: ctx => StructureProperties.chain.label_asym_id(ctx.element) === 'A'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe start the list with the chainTest, then the atomTest.
I know that order does not matter in the dictionnary, but it will be easier to get the notion that the chainTest is actually run first for efficiency purposes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, done

Comment on lines 45 to 60
structure.units.forEach(unit => {
for (let i = 0; i < unit.elements.length; i++) { // Iterate over each atom in the unit (each alpha carbon)
// Create a Loci using the unit and the index of the atom
const indices = OrderedSet.ofSingleton(i as StructureElement.UnitIndex);
const loci = StructureElement.Loci(struct, [{unit, indices: indices}]);
// Create a location to retrieve the atom's compID (residue name)
const location = StructureElement.Location.create(struct, unit, unit.elements[i]);
const resName = StructureProperties.atom.auth_comp_id(location);
// Add label to the loci
plugin.managers.structure.measurement.addLabel(loci, {labelParams: {
// Set label parameters, in this case the text and size
customText: 'Alpha Carbon '+resName,
textSize: 1
}});
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works but is inefficient, probably because it's creating 140+ representations serially. Most of the code I've stumbled upon is using special construct with a LabeProvider.
I haven't dug out more, but I suspect this is actually an antipattern.

Comment on lines 42 to 43
// A StructureSelection is a Singleton if each iteration only adds a single element (atom)
// A StructureSelection is a Sequence if any iteration adds multiple elements (atoms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A StructureSelection is made of Singletons if there is no grouping (e.g. when using Queries.generators.atoms) or when every group only contains a single element (atom). Matched elements are merged in a single unit within a unique structure in the StructureSelection var.
A StructureSelection is a Sequence if it contains at least one group with more than one element. Each group is its own Structure object in the structures array property of the StructureSelection var.
In this query, Queries.generators.chains creates a grouping at the chain level, that contains elements from the entire chains: selection is of type StructureSelection.Sequence.

// A StructureSelection is a Singleton if each iteration only adds a single element (atom)
// A StructureSelection is a Sequence if any iteration adds multiple elements (atoms)
const loci: Loci[] = [];
if (StructureSelection.isSingleton(selection)) { // Singleton if each chain only has 1 atom
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Singletons if grouping by atoms (no grouping)

// A StructureSelection is a Sequence if any iteration adds multiple elements (atoms)
const loci: Loci[] = [];
if (StructureSelection.isSingleton(selection)) { // Singleton if each chain only has 1 atom
// Iterate over each Unit in the selection and create a Loci for each Unit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterate over each Unit in the structure. By default the units in the struct initial object were segmented by chain identifiers. Each Unit corresponds to a distinct chain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't take it for granted that each unit corresponds to a whole or single chain. Best to use selections!

// Iterate over each Unit in the selection and create a Loci for each Unit
selection.structure.units.forEach(unit => {
// Create an OrderedSet of indicies for the length of unit.elements
const indices = OrderedSet.ofBounds(0 as StructureElement.UnitIndex, unit.elements.length as StructureElement.UnitIndex)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const indices = OrderedSet.ofBounds(0, unit.elements.length)

Comment on lines +54 to +57
// Iterate over each Structure in the selection and create a Loci for each SubStructure
selection.structures.forEach(structure => {
loci.push(Structure.toSubStructureElementLoci(struct, structure));
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Iterate over each Structure in the selection (one per chain) and create a Loci
...
loci.push(Structure.toStructureElementLoci(structure));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a label structure representation, which can do many use-cases much more efficiently (and happy to see more params added as needed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants