Project

General

Profile

Bug #2533

GROMACS fails to build with lmfit-7.0

Added by Christoph Junghans over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
analysis tools
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

lmfit-7 is out, but gromacs fails to build with it:

[ 32%] Building CXX object src/gromacs/CMakeFiles/libgromacs.dir/correlationfunctions/gmx_lmcurve.cpp.o
cd /home/junghans/rpmbuild/BUILD/gromacs-2018.1/serial/src/gromacs && /usr/lib64/ccache/c++  -DGMX_DOUBLE=0 -DHAVE_CONFIG_H -DUSE_STD_INTTYPES_H -Dlibgromacs_EXPORTS -I/home/junghans/rpmbuild/BUILD/gromacs-2018.1/serial/src -isystem /home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/external/thread_mpi/include -I/home/junghans/rpmbuild/BUILD/gromacs-2018.1/src  -msse2   -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -std=c++11   -DNDEBUG -funroll-all-loops -fexcess-precision=fast   -fPIC    -Wno-comment -fopenmp -o CMakeFiles/libgromacs.dir/correlationfunctions/gmx_lmcurve.cpp.o -c /home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp
/home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp: In function 'void gmx_lmcurve(int, double*, int, const double*, const double*, const double*, double (*)(double, const double*), const lm_control_struct*, lm_status_struct*)':
/home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp:84:30: error: invalid conversion from 'const void*' to 'const double*' [-fpermissive]
     lmmin(n_par, par, m_dat, (const void*)&data, lmcurve_evaluate,
                              ^~~~~~~~~~~~~~~~~~
/home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp:84:50: error: invalid conversion from 'void (*)(const double*, int, const void*, double*, int*)' to 'const void*' [-fpermissive]
     lmmin(n_par, par, m_dat, (const void*)&data, lmcurve_evaluate,
                                                  ^~~~~~~~~~~~~~~~
/home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp:85:11: error: cannot convert 'const lm_control_struct*' to 'void (*)(const double*, int, const void*, double*, int*)'
           control, status);
           ^~~~~~~
In file included from /home/junghans/rpmbuild/BUILD/gromacs-2018.1/src/gromacs/correlationfunctions/gmx_lmcurve.cpp:47:
/usr/include/lmmin.h:35:12: note:   initializing argument 6 of 'void lmmin(int, double*, int, const double*, const void*, void (*)(const double*, int, const void*, double*, int*), const lm_control_struct*, lm_status_struct*)'
     void (*evaluate)(
     ~~~~~~~^~~~~~~~~~
         const double* par, const int m_dat, const void* data,
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         double* fvec, int* userbreak),
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [src/gromacs/CMakeFiles/libgromacs.dir/build.make:4337: src/gromacs/CMakeFiles/libgromacs.dir/correlationfunctions/gmx_lmcurve.cpp.o] Error 1
make[2]: Leaving directory '/home/junghans/rpmbuild/BUILD/gromacs-2018.1/serial'
make[1]: *** [CMakeFiles/Makefile2:2702: src/gromacs/CMakeFiles/libgromacs.dir/all] Error 2
make[1]: Leaving directory '/home/junghans/rpmbuild/BUILD/gromacs-2018.1/serial

Associated revisions

Revision f3410c30 (diff)
Added by Mark Abraham over 1 year ago

Bump lmfit support to version 7

The breaking API change means that distributions will not be able to
build GROMACS reliably with support for lmfit of arbitrary versions.
Nor does lmfit provide any support for querying the version.

Updated the FindLmfit and other cmake code to make better use of
modern cmake idioms for managing build targets. lmfit support is now
handled by a tri-state GMX_USE_LMFIT cmake variable, which defaults to
using the bundled version. When building without lmfit support, tools
that call such fitting will now issue a fatal error.

Reorganized aspects of gmx_lmcurve to better encapsulate the
dependency, now that we have to support the absence of lmfit.

Updated install guide and COPYING file.

Refs #2533

Change-Id: I6b14e08be216f803326b0ad9215b8d89bb0962c1

History

#1 Updated by Mark Abraham over 1 year ago

  • Assignee changed from Mark Abraham to David van der Spoel

Version 7 changed the API. http://apps.jcns.fz-juelich.de/doku/sc/lmfit

Release-2018 bundles 6.1 but the find module for external lmfit permits 6.1 or higher. We should likely require either exactly 6.1, or [6.1, 7) in cmake/FindLmfit.cmake and adapt docs/install-guide/index.rst accordingly.

master can either do that or decide to do the same with version 7.

#2 Updated by Christoph Junghans over 1 year ago

Wearing my Fedora hat, I would prefer if it would use lmfit-7 as Fedora is basically waiting for gromacs to support that version: https://src.fedoraproject.org/rpms/lmfit/pull-request/2.
As for all distributions using a bundled libraries are not an option: https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries.

#3 Updated by Mark Abraham over 1 year ago

It's not exactly required for Fedora to choose to only permit packages to link with lmfit-7 when there are existing dependencies on lmfit-6.1. It would be totally reasonable for GROMACS to say that we developed 2018 branch against 6.1 and that's the only one we'll support as an external dependency.

If this is the kind of thing we can expect from accepting contributions that require external dependencies on projects with no intent to have a stable API, then either
  • we won't be able to accept such contributions, or
  • we won't have distributions bundle GROMACS unless their packaging and distribution system can cope with the possibility of different versions being installed, or
  • that such contributions also have to permit GROMACS to build without the dependency, and degrade gracefully with useful error messages and accurate documentation.

#4 Updated by Mark Abraham over 1 year ago

lmfit doesn't even have a way for a find_package to tell its version unless you can portably determine an SOVERSION. Sure, we can customize our linking test to detect that the version has changed, but that is brittle and still means we have to wrap the difference in gmx_lmcurve.cpp.

Christoph, can Fedora install lmfit-6.x for a user who chooses GROMACS 2018.x? Obviously that could clash with some other package wanting some other lmfit, but that's a feasible way to run an unstable distro.

Otherwise, I think we need my third option; that GMX_EXTERNAL_LMFIT should change name and behaviour to reflect the three possibilties
  • (default) link with the bundled lmfit
  • link with a compatible external lmfit
  • don't link with lmfit at all

#5 Updated by Christoph Junghans over 1 year ago

That last version sound like a good option!

Re, the fedora question, once you fixed it in master, I can try to backport the patch just for Gromacs-2018 on Fedora.

#6 Updated by Erik Lindahl over 1 year ago

In hindsight, it might have been a bad decision to allow lmfit in the first place.

There is at least (?) one version that is buggy, but since at least the previous API completely lacks any version information, there isn't even any way for us to detect whether the installed library is correct or not.

#7 Updated by Gerrit Code Review Bot over 1 year ago

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

#8 Updated by Mark Abraham over 1 year ago

  • Status changed from New to Feedback wanted

I think release-2018 works as intended, except that there's no easy way to remedy that it can find an unsuitable later version of lmfit. If Fedora has to use lmfit 7, then I hope Christoph can find a useful small patch from the master branch change i have uploaded. That forces master branch to depend only on lmfit 7, but lmfit could be more helpful about this, e.g make it possible to determine the version.

So unless there's other ideas I will declare this resolved (ie won't fix) from the POV of GROMACS 2018.2

#9 Updated by Christoph Junghans over 1 year ago

I just backported the patch to 2018.1, so merging support for lmfit-7.0 in master should be fine.

#10 Updated by David van der Spoel over 1 year ago

Sorry for the mess and not helping out a lot.

Might a fourth version be to go back to some intermediate solution that we had for a short while, namely to rename the lmfit routines to gmx_lmfit and ignore external libraries?

#11 Updated by Mark Abraham over 1 year ago

David van der Spoel wrote:

Sorry for the mess and not helping out a lot.

Might a fourth version be to go back to some intermediate solution that we had for a short while, namely to rename the lmfit routines to gmx_lmfit and ignore external libraries?

Distros will not package things if they have a bundled dependency, so we need either to be able to link with the dependency from another package, or build without the dependency (or both).

#12 Updated by Mark Abraham over 1 year ago

  • Status changed from Feedback wanted to Rejected

There's no plans to "fix" this in release-2018, but Christoph plans to backport https://gerrit.gromacs.org/7985 to suit the Fedora needs. Hopefully any other distro will find that acceptable also.

#13 Updated by Mark Abraham over 1 year ago

  • Status changed from Rejected to Closed

#14 Updated by Mark Abraham over 1 year ago

  • Subject changed from Gromacs fails to build with lmfit-7.0 to GROMACS fails to build with lmfit-7.0
  • Assignee changed from David van der Spoel to Mark Abraham
  • Target version changed from 2018.2 to 2019

Updated to bundle lmfit 7, default to build it, but permit to build without it or with an external version of it.

Also available in: Atom PDF