Skip to content

Commit

Permalink
Merge pull request #90 from OpenBioSim/backport_2023.3.1_final
Browse files Browse the repository at this point in the history 
Backported all of the changes to main for 2023.3.1
  • Loading branch information
@chryswoods
chryswoods authored Jul 27, 2023
2 parents 85e3aa3 + 77b7e1f commit 0febf8f
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 21 deletions.
41 changes: 32 additions & 9 deletions corelib/src/libs/SireIO/filetrajectory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,20 @@ SIREIO_EXPORT QDataStream &operator>>(QDataStream &ds, FileTrajectory &file)
return ds;
}

FileTrajectory::FileTrajectory() : TrajectoryData()
FileTrajectory::FileTrajectory()
: TrajectoryData(), last_loaded_frame_index(-1)
{
}

FileTrajectory::FileTrajectory(const MoleculeParser &p) : TrajectoryData(), parser(p)
FileTrajectory::FileTrajectory(const MoleculeParser &p)
: TrajectoryData(), parser(p), last_loaded_frame_index(-1)
{
}

FileTrajectory::FileTrajectory(const FileTrajectory &other) : TrajectoryData(other), parser(other.parser)
FileTrajectory::FileTrajectory(const FileTrajectory &other)
: TrajectoryData(other), parser(other.parser),
last_loaded_frame(other.last_loaded_frame),
last_loaded_frame_index(other.last_loaded_frame_index)
{
}

Expand Down Expand Up @@ -178,25 +183,43 @@ Frame FileTrajectory::getFrame(int i) const
{
i = SireID::Index(i).map(this->nFrames());

// It is often the case that we will repeatedly load the same frame
// Check for this here
QMutexLocker lkr(const_cast<QMutex *>(&(this->frame_mutex)));
FileTrajectory *nonconst_this = const_cast<FileTrajectory *>(this);

if (i == last_loaded_frame_index)
{
return this->last_loaded_frame;
}

// Ok - we are loading a different frame - see if this has been cached
auto cached_data = CentralCache::get(QString("FileTrajectory::%1::%2").arg(qint64(this)).arg(i));

QMutexLocker lkr(cached_data->mutex());
QMutexLocker lkr2(cached_data->mutex());

if (not cached_data->isEmpty())
{
try
{
return cached_data->data().value<Frame>();
nonconst_this->last_loaded_frame = cached_data->data().value<Frame>();
lkr2.unlock();
nonconst_this->last_loaded_frame_index = i;
return this->last_loaded_frame;
}
catch (...)
{
}
}

// something went wrong - read the frame again
auto frame = parser.read().getFrame(i);
cached_data->setData(QVariant::fromValue<Frame>(frame), frame.numBytes());
return frame;
// something went wrong - either the frame was corrupt or we
// haven't loaded it yet
nonconst_this->last_loaded_frame = parser.read().getFrame(i);
nonconst_this->last_loaded_frame_index = i;
cached_data->setData(QVariant::fromValue<Frame>(this->last_loaded_frame),
this->last_loaded_frame.numBytes());
lkr2.unlock();
return this->last_loaded_frame;
}

bool FileTrajectory::isEditable() const
Expand Down
4 changes: 4 additions & 0 deletions corelib/src/libs/SireIO/filetrajectory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ namespace SireIO

private:
MoleculeParserPtr parser;

QMutex frame_mutex;
SireMol::Frame last_loaded_frame;
int last_loaded_frame_index;
};

} // namespace SireIO
Expand Down
63 changes: 56 additions & 7 deletions corelib/src/libs/SireIO/pdb2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2263,9 +2263,12 @@ int PDB2::parseWholeMolecule(const SireMol::Molecule &molecule, QVector<QString>
PDBAtom atom(atoms(i), is_ter[i], map, errors);
last_atom = atom;

// Set the serial number.
atom.setSerial(1 + iline++);

// Generate a PDB atom data record.
auto record = atom.toPDBRecord();
record.replace(6, 5, QString::number(1 + iline++).rightJustified(5, ' '));
record.replace(6, 5, QString::number(atom.getNumber()).rightJustified(5, ' '));
last_record = record;

// If this record follows a TER, yet belongs to the same chain, then
Expand All @@ -2280,25 +2283,42 @@ int PDB2::parseWholeMolecule(const SireMol::Molecule &molecule, QVector<QString>
atom_lines.append(record);
prev_ter = false;

// Work out the serial number for the TER record.
auto ter_serial = 1 + atom.getNumber();

// Cap serial number to formatting width.
if (ter_serial > 99999)
ter_serial = 99999;

// Add a TER record for this atom.
if (is_ter[i])
{
atom_lines.append(QString("TER %1 %2 %3\%4\%5")
.arg(iline + 1, 5)
.arg(ter_serial, 5)
.arg(record.mid(17, 3))
.arg(record.at(21))
.arg(record.mid(22, 4))
.arg(record.at(26)));

prev_ter = true;
prev_chain = atom.getChainID();
iline++;
}
}
}

// Now append all of the post-TER records.
for (auto &line : post_ter_lines)
atom_lines.append(line.replace(6, 5, QString::number(1 + iline++).rightJustified(5, ' ')));
{
// Work out the serial number.
auto serial = 1 + iline++;

// Cap serial number to formatting width.
if (serial > 99999)
serial = 99999;

atom_lines.append(line.replace(6, 5, QString::number(serial).rightJustified(5, ' ')));
}

return iline;
}
Expand Down Expand Up @@ -2440,9 +2460,12 @@ int PDB2::parseMolecule(const SireMol::MoleculeView &sire_mol, QVector<QString>
PDBAtom atom(molecule.atom(atomidx), is_ter[i], map, errors);
last_atom = atom;

// Set the serial number.
atom.setSerial(1 + iline++);

// Generate a PDB atom data record.
auto record = atom.toPDBRecord();
record.replace(6, 5, QString::number(1 + iline++).rightJustified(5, ' '));
record.replace(6, 5, QString::number(atom.getNumber()).rightJustified(5, ' '));
last_record = record;

// If this record follows a TER, yet belongs to the same chain, then
Expand All @@ -2457,18 +2480,26 @@ int PDB2::parseMolecule(const SireMol::MoleculeView &sire_mol, QVector<QString>
atom_lines.append(record);
prev_ter = false;

// Work out the serial number for the TER record.
auto ter_serial = 1 + atom.getNumber();

// Cap serial number to formatting width.
if (ter_serial > 99999)
ter_serial = 99999;

// Add a TER record for this atom.
if (is_ter[i])
{
atom_lines.append(QString("TER %1 %2 %3\%4\%5")
.arg(iline + 1, 5)
.arg(ter_serial, 5)
.arg(record.mid(17, 3))
.arg(record.at(21))
.arg(record.mid(22, 4))
.arg(record.at(26)));

prev_ter = true;
prev_chain = atom.getChainID();
iline++;
}
}
}
Expand All @@ -2479,12 +2510,21 @@ int PDB2::parseMolecule(const SireMol::MoleculeView &sire_mol, QVector<QString>
if (last_record.isEmpty())
atom_lines.append("TER");
else
{
// Work out the serial number for the TER record.
auto ter_serial = 1 + iline++;

// Cap serial number to formatting width.
if (ter_serial > 99999)
ter_serial = 99999;

atom_lines.append(QString("TER %1 %2 %3\%4\%5")
.arg(iline + 1, 5)
.arg(ter_serial, 5)
.arg(last_record.mid(17, 3))
.arg(last_record.at(21))
.arg(last_record.mid(22, 4))
.arg(last_record.at(26)));
}

prev_ter = true;
prev_chain = last_atom.getChainID();
Expand All @@ -2493,7 +2533,16 @@ int PDB2::parseMolecule(const SireMol::MoleculeView &sire_mol, QVector<QString>

// Now append all of the post-TER records.
for (auto &line : post_ter_lines)
atom_lines.append(line.replace(6, 5, QString::number(1 + iline++).rightJustified(5, ' ')));
{
// Work out the serial number.
auto serial = 1 + iline++;

// Cap serial number to formatting width.
if (serial > 99999)
serial = 99999;

atom_lines.append(line.replace(6, 5, QString::number(serial).rightJustified(5, ' ')));
}

return iline;
}
Expand Down
16 changes: 14 additions & 2 deletions corelib/src/libs/SireMol/selectorm.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,21 @@ namespace SireMol
vws.detach();
const int n = vws.count();

// make sure that the frame has been loaded into the cache
// before we loop in parallel - this will stop contention
// from threads that are copying data from the thread that
// wants to load the frame
if (n == 0)
return;

vws[0].loadFrame(frame, map);

if (n == 1)
return;

if (_usesParallel(vws, map))
{
tbb::parallel_for(tbb::blocked_range<int>(0, n), [&](tbb::blocked_range<int> r)
tbb::parallel_for(tbb::blocked_range<int>(1, n), [&](tbb::blocked_range<int> r)
{
for (int i = r.begin(); i < r.end(); ++i)
{
Expand All @@ -387,7 +399,7 @@ namespace SireMol
}
else
{
for (int i = 0; i < n; ++i)
for (int i = 1; i < n; ++i)
{
vws[i].loadFrame(frame, map);
}
Expand Down
15 changes: 13 additions & 2 deletions doc/source/changelog.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.
`2023.3.1 <https://github.com/openbiosim/sire/compare/2023.2.3...2023.3.1>`__ - July 2023
-----------------------------------------------------------------------------------------

* Fixed a bug in ``analyse_freenrg`` which produced incorrect TI results
when not all lambda windows were run for equal lengths of time.

* Make sure atom serial number in PDB files are capped when renumbering when
TER records are present.

* Fixed a bug in the AmberRst parser where velocities were written with the wrong
unit (A ps-1 instead of AKMA time). Also added the correct labels to the AmberRst file.

Expand All @@ -24,8 +30,13 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.
* Fixed a bug in the writing of DCD headers, meaning that the files couldn't be read
by other DCD reader software (written non-compliant header)

* Fixed a bug in ``analyse_freenrg`` which produced incorrect TI results
when not all lambda windows were run for equal lengths of time.
* Fixed a bug in the trajectory measure code, where the ProgressBar class was
not being properly imported (`fix_88 <https://github.com/OpenBioSim/sire/issues/88>`__).

* Fixed a deadlock in the file trajectory loading code. This was because multiple threads
trying to read the same frame lead to starvation of the thread that had progressed to
read the frame. Now a single thread loads the frame, with subsequent threads using
this cached load (`fix_88 <https://github.com/OpenBioSim/sire/issues/88>`__).

* Optimised the speed of viewing large molecules in NGLView, plus of searching
for water molecules. Added a new ``is_water`` function. Optimised the
Expand Down
2 changes: 1 addition & 1 deletion src/sire/mol/_trajectory.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ def _custom_measures(self, func, to_pandas):
colnames.append(key)
columns.append(np.zeros(nframes, dtype=float))

from ..utils import Console
from ..base import ProgressBar

with ProgressBar(
total=nframes, text="Looping through frames"
Expand Down

0 comments on commit 0febf8f

Please sign in to comment.