Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions corelib/src/libs/SireIO/amberprm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,14 @@ QVector<QVector<int>> indexCMAPs(const QVector<qint64> &cmaps, const QVector<int
for (int i = 0; i < cmaps.count(); i += 6)
{
// format is atom0-atom1-atom2-atom3-atom4-parameter
int mol0 = atom_to_mol_data[cmaps_data[i]]; // DO NOT DIVIDE BY THREE
int mol1 = atom_to_mol_data[cmaps_data[i + 1]]; // THIS IS THE RAW ATOM INDEX
int mol2 = atom_to_mol_data[cmaps_data[i + 2]];
int mol3 = atom_to_mol_data[cmaps_data[i + 3]];
int mol4 = atom_to_mol_data[cmaps_data[i + 4]];
// CMAP indices are plain 1-based atom numbers (unlike bonds/angles/dihedrals
// which are 3×(atom_0based)). Subtract 1 to convert to the 0-based index
// used by atom_to_mol.
int mol0 = atom_to_mol_data[cmaps_data[i] - 1];
int mol1 = atom_to_mol_data[cmaps_data[i + 1] - 1];
int mol2 = atom_to_mol_data[cmaps_data[i + 2] - 1];
int mol3 = atom_to_mol_data[cmaps_data[i + 3] - 1];
int mol4 = atom_to_mol_data[cmaps_data[i + 4] - 1];

if (mol0 != mol1 or mol0 != mol2 or mol0 != mol3 or mol0 != mol4)
throw SireIO::parse_error(
Expand Down Expand Up @@ -2249,7 +2252,10 @@ std::tuple<QVector<qint64>, QHash<CMAPParameter, qint64>> getCMAPData(const Ambe
QHash<CMAPParameter, qint64> param_to_idx;
QVector<qint64> cmap_idxs;

start_idx /= 3;
// NOTE: start_idx is in atom units (0-based count of atoms before this
// molecule). CMAP indices in the prmtop file are plain 1-based atom
// numbers (NOT multiplied by 3 as bonds/angles/dihedrals are).
// Do NOT divide by 3 here.

const auto info = params.info();
const auto cmaps = params.cmapFunctions().parameters();
Expand Down
4 changes: 4 additions & 0 deletions doc/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.

* Please add an item to this CHANGELOG for any new features or bug fixes when creating a PR.

* Fixed a bug in the AMBER prmtop writer where CMAP atom indices were calculated
incorrectly for systems containing more than one molecule with CMAP terms (e.g.
multi-chain glycoproteins).

`2025.4.0 <https://github.com/openbiosim/sire/compare/2025.3.0...2025.4.0>`__ - February 2026
---------------------------------------------------------------------------------------------

Expand Down
5 changes: 5 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ def amber_cmap():
return sr.load_test_files("amber_cmap.prm7")


@pytest.fixture(scope="session")
def multichain_cmap():
return sr.load_test_files("multichain_cmap.prm7", "multichain_cmap.rst7")


@pytest.fixture(scope="session")
def gromacs_cmap():
return sr.load_test_files("1aki.gro", "1aki.top")
Expand Down
47 changes: 47 additions & 0 deletions tests/io/test_ambercmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,53 @@ def test_amber_cmap(tmpdir, amber_cmap):
assert line1 == line2


def test_amber_multichain_cmap(tmpdir, multichain_cmap):
"""Regression test for multi-chain CMAP write bug.

When a topology has more than one molecule with CMAP terms, the write path
previously applied a spurious '/= 3' to the per-molecule atom offset,
producing wrong global atom indices for molecules 2, 3, … . The re-parse
of the generated text then raised:
"there is a cmap between more than one different molecule"
"""
mols = multichain_cmap

# Collect per-molecule CMAP term counts indexed by position in the
# molecule list. There must be at least two molecules with CMAP
# to trigger the multi-chain bug.
cmap_counts = {}
for i, mol in enumerate(mols.molecules()):
if mol.has_property("cmap"):
cmap_counts[i] = len(mol.property("cmap").parameters())

assert (
len(cmap_counts) >= 2
), "Expected at least two molecules with CMAP terms in this topology"

dir = tmpdir.mkdir("test_amber_multichain_cmap")

# Write the topology back to file. (This is where the bug used to trigger.)
f = sr.save(mols, dir.join("output"), format="prm7")

# Reead the file back in.
mols2 = sr.load(f)

# Each molecule must still have the same number of CMAP terms after the
# roundtrip.
for i, count in cmap_counts.items():
mol2 = mols2[i]
assert mol2.has_property(
"cmap"
), f"Molecule at index {i} lost its cmap property after roundtrip"
count2 = len(mol2.property("cmap").parameters())
assert (
count2 == count
), f"Molecule at index {i}: CMAP count changed from {count} to {count2}"

# Verify a second write also succeeds without error.
sr.save(mols2, dir.join("output2"), format="prm7")


def test_amber_cmap_grotop(tmpdir, amber_cmap):
"""Testing reading and writing from amber to gromacs and back to amber."""
mols = amber_cmap.clone()
Expand Down