Project

General

Profile

Bug #488

Recent Changes to commrec.h break FAHcore

Added by Kyle Beauchamp over 9 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Erik Lindahl
Category:
mdrun
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

I've been regularly re-building FAH cores based on the gromacs git, so that we can have a working FAH core based on the shiny 4.5 release. However, some very recent changes have broken the current threading model in FAH-only code.

These changes lead to the following build error:

5>Y:\fah_windows_build\gromacs-aug1\src\kernel\md.c(2125): error: struct "<unnamed>" has no field "nthreads"
5> int nthreads=(cr->nthreads==0 ? 1 : cr->nthreads);

The problem is that the recent changes to commrec.h have removed the nthreads variable from the t_commrec struct. The changes occurred a few days ago:

4 days ago Berk commented out nthreads and threadid in t_commrec to... blob | commitdiff

http://repo.or.cz/w/gromacs.git/commit/1e5db900e7a298ecce04e7d91949bef49f41519f

I'll try to figure out a workaround. I guess the main issue on my end is figuring out how the new code is supposed to work. For now, I just wanted to bring this to your attention. I suspect that keeping FAH-gromacs code compliant with the rest of gromacs is now (partly or fully) my responsibility, so I'll look into this further.

History

#1 Updated by Erik Lindahl over 9 years ago

Hi,

Those variables were killed to remove confusion. The current domain decomposition threading works by using Sander's thread-based MPI wrapper, so the commrec considers each thread a 'node' (or, think: decomposition volume part), while the old variables referred to multiple threads working on the same volume.

#2 Updated by Berk Hess over 9 years ago

It seems that particular line is useless, since the variable is never used.
Shall I remove the unused code?

Berk

#3 Updated by Kyle Beauchamp over 9 years ago

You're right, that variable is never used, so we might as well delete it. I should have checked noticed that earlier.

#4 Updated by Berk Hess over 9 years ago

I removed the unused code.

Berk

Also available in: Atom PDF