Project

General

Profile

Bug #1453

tng_io installation issues

Added by Teemu Murtola over 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Category:
build system
Target version:
Affected version - extra info:
dev versions of 5.0
Affected version:
Difficulty:
uncategorized
Close

Description

There are some issues with how the Gromacs build system handles the tng libraries (and some issues in the tng itself). Logging them here so that they don't get forgotten and may get proper treatment at some point.

  • As it is now, it is impossible to use tng_io as a proper library (and not as a source distribution, like is used for Gromacs), since it only installs include/tng_io.h. But tng_io.h contains a #include "tng_io_fwd.h", which fails to compile from the installed headers.
  • It is impossible to use a tng_io library that is already present on a system. If we want to treat the tng_io as a first-class citizen that other projects can also use for trajectory I/O, wouldn't it make sense to actually make it such that we can use it ourselves that way as well?
  • The Gromacs build system installs libtng* and the headers unconditionally to the installation prefix, overwriting any other version the system may already have there. There are several issues with this:
    • If one compiles Gromacs with multiple suffixes, with different options for each, and installs them, the tng libraries are compiled again for each case, but only the ones from the last installation actually remains on the system, and gets used by all the different Gromacs versions. Hopefully, most Gromacs compilation options don't influence tng_io and the libraries will be compatible with each other, but there can be corner cases that mysteriously break because of this.
    • If one wants to install to a system-wide location (e.g., /usr/local/), it can silently break all other applications on the system that are using tng_io, if they expect a different tng_io version being present (or because Gromacs compiled it with some weird options that don't work for other applications).
    • Even if the binaries work, currently the headers break because of the first bullet above, making it possible to compile any other applications against the installed tng_io library.

If we want to keep building the tng libraries as part of our build, we should make it behave nicely with a pre-existing tng installation (at least providing an option to use that, and not silently breaking it) if it is a realistic plan that tng will be used by other applications. Alternatively, we should compile and link the tng components directly into libgromacs, like is done with GMX_BUILD_OWN_FFTW and with thread_mpi, so that we don't need to install anything related to tng.

It depends on the plans for the independent tng libraries on how important these are to fix.


Related issues

Related to GROMACS - Bug #1452: trxio.h cannot be linked by external packages anymoreClosed03/05/2014
Related to GROMACS - Bug #1520: tng include/library/linking problemsClosed06/10/2014

Associated revisions

Revision ef40a754 (diff)
Added by Magnus Lundborg over 6 years ago

Update TNG build system

GROMACS can now find and use a copy of TNG installed on the system.
The default is still to compile the version shipped with GROMACS.
That compilation is now handled using the BuildTNG.cmake script, so
there is no duplication between the GROMACS and TNG CMake code that
builds TNG, and the result is fully portable with all supported
environments and CMake versions.

Moved TNG handling into src/gromacs/CMakeLists.txt where it is
actually used.

Bumped the version of the TNG library to 1.6. Corresponds to commit
ba9aea42b01 in the TNG repository.

Renamed md5 things with GROMACS prefixes, so that there is no internal
name clash with the md5 things in TNG, and no client of libgromacs can
get unexpected behaviour.

Updated GMX_USE_TNG=off to work, and silenced unused-parameter
warnings associated with that setting.

Refs #1453 (partial fix)
Fixes #1520

Change-Id: I7ac9b578e45b8e75c67c9df9440eed968a3a9371

History

#1 Updated by Teemu Murtola over 6 years ago

  • Related to Bug #1452: trxio.h cannot be linked by external packages anymore added

#2 Updated by Mark Abraham over 6 years ago

Thanks for the report. I'd noticed some of these myself, and we should certainly work on fixing them!

#3 Updated by Roland Schulz over 6 years ago

I think the tng headers should be installed in some subfolder of gromacs when it is installed as a part of gromacs.

#4 Updated by Gerrit Code Review Bot over 6 years ago

Gerrit received a related patchset '1' for Issue #1453.
Uploader: Mark Abraham ()
Change-Id: I63477b5d159be7316ba9359a214b7a7145e2d426
Gerrit URL: https://gerrit.gromacs.org/3570

#5 Updated by Erik Lindahl over 6 years ago

  • Priority changed from Normal to High
  • Target version set to 5.0

#6 Updated by Mark Abraham over 6 years ago

Teemu Murtola wrote:

There are some issues with how the Gromacs build system handles the tng libraries (and some issues in the tng itself). Logging them here so that they don't get forgotten and may get proper treatment at some point.

  • As it is now, it is impossible to use tng_io as a proper library (and not as a source distribution, like is used for Gromacs), since it only installs include/tng_io.h. But tng_io.h contains a #include "tng_io_fwd.h", which fails to compile from the installed headers.

Needs work from Magnus. Suggestions on correct form at #1490.

  • It is impossible to use a tng_io library that is already present on a system. If we want to treat the tng_io as a first-class citizen that other projects can also use for trajectory I/O, wouldn't it make sense to actually make it such that we can use it ourselves that way as well?

Indeed. A FindTNG.cmake would be easy to write once the above is resolved; then we can have the bundled TNG as fallback.

  • The Gromacs build system installs libtng* and the headers unconditionally to the installation prefix, overwriting any other version the system may already have there. There are several issues with this:
    • If one compiles Gromacs with multiple suffixes, with different options for each, and installs them, the tng libraries are compiled again for each case, but only the ones from the last installation actually remains on the system, and gets used by all the different Gromacs versions. Hopefully, most Gromacs compilation options don't influence tng_io and the libraries will be compatible with each other, but there can be corner cases that mysteriously break because of this.

My fix resolves this by not installing TNG libraries (and header) when built internally. I suspect not installing the header breaks the other headers that we do install, but have not tested that yet.

  • If one wants to install to a system-wide location (e.g., /usr/local/), it can silently break all other applications on the system that are using tng_io, if they expect a different tng_io version being present (or because Gromacs compiled it with some weird options that don't work for other applications).

As above, I think we should not install TNG when built internally.

  • Even if the binaries work, currently the headers break because of the first bullet above, making it possible to compile any other applications against the installed tng_io library.

If we want to keep building the tng libraries as part of our build, we should make it behave nicely with a pre-existing tng installation (at least providing an option to use that, and not silently breaking it) if it is a realistic plan that tng will be used by other applications. Alternatively, we should compile and link the tng components directly into libgromacs, like is done with GMX_BUILD_OWN_FFTW and with thread_mpi, so that we don't need to install anything related to tng.

Yep

It depends on the plans for the independent tng libraries on how important these are to fix.

#7 Updated by Gerrit Code Review Bot over 6 years ago

Gerrit received a related patchset '1' for Issue #1453.
Uploader: Magnus Lundborg ()
Change-Id: I4b2a3d126972d0752df77e4aa6312d4c36c37bec
Gerrit URL: https://gerrit.gromacs.org/3594

#8 Updated by Gerrit Code Review Bot over 6 years ago

Gerrit received a related patchset '1' for Issue #1453.
Uploader: Rossen Apostolov ()
Change-Id: I4b2a3d126972d0752df77e4aa6312d4c36c37bec
Gerrit URL: https://gerrit.gromacs.org/3596

#9 Updated by Mark Abraham over 6 years ago

  • Status changed from New to In Progress
  • Priority changed from High to Normal
  • Target version changed from 5.0 to 5.x

I think the patches in and through Gerrit have dealt well enough for 5.0. We're not yet aware of any uptake of TNG except perhaps in VMD, so dealing with the prospect of running into an installed TNG is not yet a priority. It would be nice to have and run FindTNG.cmake and use it if it was installed, but in practice having enough binary stability to use a new TNG with an old GROMACS is probably quite some time in the future.

It would be nice for TNG to provide a simple FindTNG.cmake (e.g. examples in GROMACS cmake/) for other applications to use, but it's not needed for GROMACS 5.0.

#10 Updated by Mark Abraham over 6 years ago

  • Related to Bug #1520: tng include/library/linking problems added

#11 Updated by Mark Abraham over 6 years ago

There's ongoing issues in TNG and the way GROMACS uses it that we must consider / fix. See discussion at https://gerrit.gromacs.org/#/c/3570/2/src/external/tng_io/src/compression/CMakeLists.txt

#12 Updated by Gerrit Code Review Bot over 6 years ago

Gerrit received a related patchset '1' for Issue #1453.
Uploader: Mark Abraham ()
Change-Id: Ibc07b8f6f194e5d15c2ac41aca4cdcd41bd0ef9e
Gerrit URL: https://gerrit.gromacs.org/3685

#13 Updated by Gerrit Code Review Bot over 6 years ago

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

#14 Updated by Gerrit Code Review Bot over 6 years ago

Gerrit received a related DRAFT patchset '1' for Issue #1453.
Uploader: Mark Abraham ()
Change-Id: I1d7e2e71363075588ea038034963c742c73800e9
Gerrit URL: https://gerrit.gromacs.org/3706

#15 Updated by Magnus Lundborg over 6 years ago

There is of course still room for improvement, but aren't all these issues fixed now?

#16 Updated by Teemu Murtola about 6 years ago

  • Status changed from In Progress to Closed
  • Target version changed from 5.x to 5.0

Yes, everything mentioned here is probably fixed now.

Also available in: Atom PDF