Project

General

Profile

Task #1862

Fully replace t_topology by gmx_mtop_t

Added by Berk Hess over 3 years ago. Updated 6 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
analysis tools
Target version:
-
Difficulty:
uncategorized
Close

Description

In grompp and mdrun gmx_mtop_t has mostly replaced t_topology. The local topology still uses t_topology, but that only uses a subset of it.
But the old and new analysis framework exclusively use t_topology. We should replace this by gmx_mtop_t. One reason is to have only a single topology format, but it is also a far more efficient way of storing information and it could also speed up selections.
One question is if it's worth the effort to replace it in the old framework. It might be better to use our effort to port old tools to the new framework.


Checklist

  • Move TopologyInformation to its own header and source file to reduce compilation coupling
  • Extend TopologyInformation to suit the needs of analysis tools not yet ported to the analysis framework (e.g. read velocities from -s, handle rvec instead as RVec, produce gmx_localtop_t where needed rather than t_topology)
  • Fix any bugs identified by converting code to new infrastructure
  • Separate TextLineWrapper from stringutil (few clients of the latter need the former, and the latter is a bit big now)
  • Convert TextWriter indenting model from get-and-set to push-and-pop, which better suits the way we do and will use it
  • Convert code for dump, check, compare, convert-tpr to use TextWriter instead of stdout, so they can have tests
  • Convert code for dump, check, compare, convert-tpr to use TopologyInformation (and use gmx_mtop_t rather than t_topology)
  • Add basic test coverage for code using readConfAndTopology
  • Convert code using readConfAndTopology to use new builder for TopologyInformation
  • Add basic test coverage for many analysis tools, e.g. checking output xvg contents with refdata (use several commits)
  • Convert legacy analysis code using e.g. fitting functionality to use ArrayRef, to prepare for using TopologyInformation
  • Convert legacy analysis code using e.g. fitting functionality to use TopologyInformation
  • Convert remaining legacy analysis code to use TopologyInformation
  • Remove t_topology and routines no longer used by legacy analysis code (e.g. read_tps_top() etc., and parts of TopologyInformation)
  • Refactor output writing from legacy analysis code to only use ArrayRef and RVec
  • Refactor legacy analysis tools to eliminate read_next_x and read_next_frame

Related issues

Related to GROMACS - Bug #2847: Index groups (-n) in selections do not work without -sClosed

Associated revisions

Revision b22c29fa (diff)
Added by Berk Hess almost 3 years ago

Add readConfAndTopology

readConfAndTopology will replace read_tps_conf, reading to gmx_mtop_t
instead of to the deprecated t_topology struct.
With requireMasses=TRUE, read_tps_conf will generate a fatal error when
an atom is not found in the mass database, as it was originally
intended, since we don't want to calculate with incorrect masses.
The availability of masses is signaled by the haveMass bool in t_atoms.
The trajectory analysis framework now works with optional masses.
This is done by not requiring masses and attempting to look up masses
from the database when they were not present in the structure file.

Refs #1862.

Change-Id: I045649fa458a08415fd319c568d15ad514a30a9f

Revision 8d63efb3 (diff)
Added by Teemu Murtola almost 3 years ago

Support concurrent t_topology and gmx_mtop_t use

Make gmx_mtop_t_to_t_topology() take a boolean flag that specifies
whether to free memory from mtop or not. This makes it possible to
convert an mtop to a t_topology and then use both in subsequent code.
This in turn makes a piecewise conversion from t_topology to gmx_mtop_t
possible.

Using these two concurrently currently always leads to memory leaks, but
these should mostly be temporary during the conversion, and can be
sorted out separately if necessary.

Part of #1862.

Change-Id: Iccd6039c2226d1dc617963878e5640bb53f09ad6

Revision 3257f555 (diff)
Added by Teemu Murtola almost 3 years ago

Wrap sel. method evaluation parameters to struct

This removes about 1/3 of t_topology instances in the selection code,
reducing the number of lines that need to be changed when switching to
gmx_mtop_t.

Also make most of the contents const.

Related to #1862.

Change-Id: Idcd71bc578a6daa4644f049a31f84e74717c258a

Revision f0003532 (diff)
Added by Teemu Murtola almost 3 years ago

Use gmx_mtop_t in selections, part 1

Make the C++ analysis framework (and insert-molecules, which is also
using selections) use a gmx_mtop_t as the initial internal format to
load the topology, and make SelectionCollection::setTopology() use an
mtop. Adapt tests. One test is disabled as it is much easier to
re-enable it after the next step in the conversion process than in this
intermediate state.

Internally, the mtop structure is still converted to t_topology for use
in lower levels and in the tool code.

This change also makes it possible to convert the analysis tools and gmx
insert-molecules to use gmx_mtop_t independently from changes in the
selection code.

Part of #1862.

Change-Id: I3ce03c6524b27f0f44168890ac1a4a491da52a4c

Revision b2b1088c (diff)
Added by Teemu Murtola almost 3 years ago

Use gmx_mtop_t in selections, part 2

Use gmx_mtop_t throughout low-level selection routines, i.e.,
centerofmass.cpp, poscalc.cpp, and indexutil.cpp. Adapt test code,
which is now using gmx_mtop_t throughout as well.

In places where gmx_mtop_t is actually accessed, the changes are as
local as possible. In most cases, some additional restructuring could
give better performance and/or much clearer code, but that is outside
the scope of this change.

Part of #1862.

Change-Id: Icc99432bddec04a325aef733df56571d709130fb

Revision c76765d3
Added by Teemu Murtola almost 3 years ago

Use gmx_mtop_t in selections, part 2

Use gmx_mtop_t throughout low-level selection routines, i.e.,
centerofmass.cpp, poscalc.cpp, and indexutil.cpp. Adapt test code,
which is now using gmx_mtop_t throughout as well.

In places where gmx_mtop_t is actually accessed, the changes are as
local as possible. In most cases, some additional restructuring could
give better performance and/or much clearer code, but that is outside
the scope of this change.

Part of #1862.

Change-Id: Icc99432bddec04a325aef733df56571d709130fb

Revision 748efc45 (diff)
Added by Teemu Murtola almost 3 years ago

Use gmx_mtop_t in selections, part 3

Convert remainder of the selection code to use gmx_mtop_t.

Part of #1862.

Change-Id: Icb1c4a30cc5456e64ed2d73f93ca4a7d6dbfbfbd

Revision 9a101f83 (diff)
Added by Mark Abraham about 1 year ago

Clarify the implementation of selection -ofpdb output

This prepares for future clean up to remove t_topology. That the
information in the t_pdbinfo is intended to be associated with the
t_atoms is now a bit clearer.

Refs #1862

Change-Id: I07a9796debe25f5023c229b29f0802f9e3c77065

Revision 96f8484c (diff)
Added by Mark Abraham about 1 year ago

Add infrastructure for tools cleanup

Copy and move construction and assignment are useful to have in RVec,
plus tests. These make sense to have in a simple value type, and the
default implementations are appropriate.

Note that plans to replace t_topology reading in legacy analysis tools
in favour of the approach currently in the new analysis suite will
mean we need to use more RVec and less rvec in those legacy
tools. This change prepares for that.

Fixed a comment.

Refs #1862

Change-Id: I7d0b9bbad9dc61299f6b85a3597082c4867c0202

Revision 34be89f1 (diff)
Added by Mark Abraham about 1 year ago

Split TopologyInformation into its own header

This will help provide a useful upgrade path for tools to get away
from t_topology, and eventually into the new framework.

Introduced a builder function to fill TopologyInformation from a tpr
file, which will be needed shortly for porting old tools.

Refs #1862

Change-Id: If2594cdcd3f9817ff83e7fb73208cdb5c497bb1a

Revision 264458c9 (diff)
Added by Mark Abraham 12 months ago

Change TopologyInformation implementation

This changes TopologyInformation so that we will be able to use it
also in the legacy tools, providing a better migration path for them,
as well as making progress to removing t_topology and a lot of calls
to legacy file-reading functions.

It can now lazily build and cache atom and expanded topology data
structures, re-using the gmx_localtop_t type (intended for use by the
DD code for domain-local topologies). The atoms data structure can
also be explicitly copied out, so that tools who need to modify it can
do so without necessarily incurring a performance penalty. All these
are convenient for tools to use.

The atom coordinate arrays are now maintained as std::vector, which
might want a getter overload to make rvec * for the legacy tools.

Added tests that the reading behaviour for various kinds of inputs is
as expected. Converted lysozyme.gro to pdb, added a 'B' chain ID,
generated a .top (which needed an HG for CYS) so updated pdb and gro
accordingly. Some sasa test refdata needed fixing for that minor
change.

Provided a convenience overload of gmx_rmpbc_init that takes
TopologyInformation as input, as this will frequently be used by
tools.

Extended getTopologyConf to also return velocities, which will
be needed by some tools.

Adapted the trajectoryanalysis modules to use the new approach, which
is well covered by tests.

Refs #1862

Change-Id: I2f43e62bc2d97f5e654f15c6e474b9b71d7106ec

Revision 5181f8d1 (diff)
Added by Mark Abraham 12 months ago

Replace readConformation with TopologyInformation

Removed that function and renamed its source file to reflect the only
remaining contents.

Refs #1862

Change-Id: I6616ab8930a90d33ac4a836f97c30f6bea40e29c

Revision b9e0922a (diff)
Added by David van der Spoel 8 months ago

Removed t_topology from gmx msd

Removed t_topology but needed to introduce some new code from
block.h and topologyinformation.h to make it work.

Part of #1862

Change-Id: If7f7e2ac1cbcf1f87cb1032fb7ebf8624063baef

Revision 1edec14c (diff)
Added by Paul Bauer 8 months ago

Add mtop comparison function

Added a function to directly compare gmx_mtop_t datastructures and not
go through the conversion to t_topology first when comparing tpr files.

Removed the old function.

Changed some const pointers to const references.

Refs #1862

Change-Id: I3d8c4a1274dee221231b2d4bf2719717c8a6379d

Revision 70d0bfa5 (diff)
Added by Mark Abraham 6 months ago

Revert changes to implementation of insert-molecules and solvate

This is a partial reversion of 5181f8d1, which made an inappropriate
dependency from preparation tools to TopologyInformation, which is
intended for analysis tools. This is needed in order to fix some
actual analysis tools that were broken by related changes to
TopologyInformation.

Unfortunately in 5181f8d1 readConformation was replaced with
TopologyInformation, and the former is no longer available. Instead,
readConfAndTopology is used in the same way that it was originally
used to implement readConformation. So the behaviour is reverted
even though the code looks different.

Refs #2847, #1862

Change-Id: I6e4900eea38b5c2e1b85b0d48427dabe26b09f9d

History

#1 Updated by Mark Abraham about 3 years ago

I'm happy to help with this

#2 Updated by David van der Spoel about 3 years ago

Me too, but it requires some work in the core of the analysis tools.

#3 Updated by Teemu Murtola about 3 years ago

The first thing that this would require would be file I/O routines that read a mtop instead of a t_topology from any of the supported file formats.

#4 Updated by Teemu Murtola about 3 years ago

Teemu Murtola wrote:

The first thing that this would require would be file I/O routines that read a mtop instead of a t_topology from any of the supported file formats.

More specifically, we would need a version of read_tps_conf() that returns a gmx_mtop_t. It can take a bit of thinking how to populate it, e.g., from a gro file. Once we have that, I can take a look at adapting the selection code. t_topology is there throughout, but it isn't accessed that much.

#5 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '2' for Issue #1862.
Uploader: Berk Hess ()
Change-Id: I045649fa458a08415fd319c568d15ad514a30a9f
Gerrit URL: https://gerrit.gromacs.org/5935

#6 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Teemu Murtola ()
Change-Id: Idcd71bc578a6daa4644f049a31f84e74717c258a
Gerrit URL: https://gerrit.gromacs.org/5964

#7 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Teemu Murtola ()
Change-Id: Iccd6039c2226d1dc617963878e5640bb53f09ad6
Gerrit URL: https://gerrit.gromacs.org/5969

#8 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Teemu Murtola ()
Change-Id: I3ce03c6524b27f0f44168890ac1a4a491da52a4c
Gerrit URL: https://gerrit.gromacs.org/5970

#9 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Teemu Murtola ()
Change-Id: Icc99432bddec04a325aef733df56571d709130fb
Gerrit URL: https://gerrit.gromacs.org/5977

#10 Updated by Gerrit Code Review Bot almost 3 years ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Teemu Murtola ()
Change-Id: Icb1c4a30cc5456e64ed2d73f93ca4a7d6dbfbfbd
Gerrit URL: https://gerrit.gromacs.org/6221

#11 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I07a9796debe25f5023c229b29f0802f9e3c77065
Gerrit URL: https://gerrit.gromacs.org/8068

#12 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I7d0b9bbad9dc61299f6b85a3597082c4867c0202
Gerrit URL: https://gerrit.gromacs.org/8069

#13 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~If2594cdcd3f9817ff83e7fb73208cdb5c497bb1a
Gerrit URL: https://gerrit.gromacs.org/8070

#14 Updated by Mark Abraham about 1 year ago

I added a checklist of things I have identified to make progress here. Some patches are in gerrit already.

Much of that list I have already done in a crude form, but I need to spam some more test coverage before we would feel good about changing the infrastructure behind lots of tools currently lacking tests.

I am also looking at clang-based libtooling for automating some of the transformations, e.g. from fprintf-to-stderr to writer->writeFormattedLine()

#15 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I2f43e62bc2d97f5e654f15c6e474b9b71d7106ec
Gerrit URL: https://gerrit.gromacs.org/8072

#16 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I6616ab8930a90d33ac4a836f97c30f6bea40e29c
Gerrit URL: https://gerrit.gromacs.org/8075

#17 Updated by Gerrit Code Review Bot 9 months ago

Gerrit received a related patchset '5' for Issue #1862.
Uploader: Paul Bauer ()
Change-Id: gromacs~master~I3d8c4a1274dee221231b2d4bf2719717c8a6379d
Gerrit URL: https://gerrit.gromacs.org/8785

#18 Updated by Gerrit Code Review Bot 9 months ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: David van der Spoel ()
Change-Id: gromacs~master~If7f7e2ac1cbcf1f87cb1032fb7ebf8624063baef
Gerrit URL: https://gerrit.gromacs.org/8789

#19 Updated by David van der Spoel 9 months ago

For analysis tools, we can spend effort to remove t_topology in place (as I did in a patch to gmx msd https://gerrit.gromacs.org/#/c/8789/) or port them to the new framework at once. In an ideal world we would

1. add tests for the existing code
2. port the code to the new framework

#20 Updated by Gerrit Code Review Bot 7 months ago

Gerrit received a related DRAFT patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I153f0530f7c762a4cb4b9d1af5a67b93445fd476
Gerrit URL: https://gerrit.gromacs.org/9056

#21 Updated by Mark Abraham 7 months ago

  • Related to Bug #2847: Index groups (-n) in selections do not work without -s added

#22 Updated by Gerrit Code Review Bot 7 months ago

Gerrit received a related DRAFT patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I9bdd288460cab719a69e74011befd06d2b43ac51
Gerrit URL: https://gerrit.gromacs.org/9061

#23 Updated by Gerrit Code Review Bot 6 months ago

Gerrit received a related patchset '1' for Issue #1862.
Uploader: Mark Abraham ()
Change-Id: gromacs~release-2019~I6e4900eea38b5c2e1b85b0d48427dabe26b09f9d
Gerrit URL: https://gerrit.gromacs.org/9145

Also available in: Atom PDF