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

Fixed issues related to align_selection=None #396

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

CHENFENG1020
Copy link

No description provided.

@CHENFENG1020
Copy link
Author

CHENFENG1020 commented Mar 8, 2025

BSS.Metadynamics.CollectiveVariable.RMSD raises an error when align_selection=None. However, the parameter information states that if None, the RMSD will be calculated without alignment.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): y
  • I confirm that I have permission to release this code under the GPL3 license: y

Suggested reviewers:

@lohedges @mb2055

Any additional context of information?

  1. Would it be better if setting align_selection to None, the system will be aligned using rmsd_selection?
  2. I am working on ensuring AMMo is compatible with the latest BioSimSpace. @AdeleHardie uses this function to format the reference PDB for PLUMED in AMMo, and in her example, the reference contains fewer residues. I have read RMSD reference containing protein and ligand #376 and Fix issue #376 #379 and noticed that this function previously worked when the residue numbers differed between the reference and the system. Is it necessary to enforce that the system and reference have the same number of residues (and therefore the same number of atoms)? Could we instead ensure that align_selection and rmsd_selection each have the same number of residues and atoms in both the system and the reference?

@CHENFENG1020 CHENFENG1020 reopened this Mar 8, 2025
@lohedges
Copy link
Contributor

lohedges commented Mar 8, 2025

Thanks, I'll take a look next week. The main issue is that the selections can now refer to atoms in multiple molecules, so the old logic for matching atoms from the reference to those in the system, which matched by residue index and atom name, is much more complicated. It's certainly possible, but I wasn't sure if it was worth the effort at the time. I'll see how easy it would be to implement.

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.

2 participants