Project

General

Profile

Bug #2342

Avoid yelling about thread pinning twice

Added by Erik Lindahl almost 2 years ago. Updated almost 2 years ago.

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

Description

When running without using all threads, the latest changes now produces double pinning warnings, covering 6 lines on stderr:

"
NOTE: The number of threads is not equal to the number of (logical) cores
and the -pin option is set to auto: will not pin threads to cores.
This can lead to significant performance degradation.
Consider using -pin on (and -pinoffset in case you run multiple jobs).

NOTE: Thread affinity was not set.
"


Related issues

Related to GROMACS - Bug #2345: Avoid duplicate over-subscription note+warningClosed
Related to GROMACS - Bug #2346: inconsistent behavior of mdrun -nt 1 with mdrun -nt 3 when both are undersubscribedClosed

Associated revisions

Revision 24a9d798 (diff)
Added by Berk Hess almost 2 years ago

Avoid yelling about thread pinning twice

Do not print the second, generic note about not pinning threads
when we printed a first, more detailed warnings.
Updated the threadaffinity tests for this.

Fixes #2342

Change-Id: I1fa8374d70891c3bc1caf2827210875e1c1a020f

History

#1 Updated by Szilárd Páll almost 2 years ago

Why is this "yelling" and how is worth a "high" priority bug report when the previous silly message has been telling users the affinity setting "failed" (an expected by design outcome) was not important for >1 year?

Unless this requires no code redesign (which I'm not sure it doesn't), I think this is at best a normal to low priority feature.

#2 Updated by Erik Lindahl almost 2 years ago

Because it is a new error we introduced since beta1.

#3 Updated by Berk Hess almost 2 years ago

  • Priority changed from High to Normal

This second note is the warning I toned down in https://gerrit.gromacs.org/#/c/7291/ (also reviewed by Erik). I mentioned in the discussion there that this setup with sometimes double notes is awkward, but that it requires code reorganization or passing around if we warned before to avoid double notes. Also the tests would need to be updated for that. (Note that the situation is much worse in release-2016 without my change.)

#4 Updated by Erik Lindahl almost 2 years ago

In that case the obvious solution is to move the first note so it only goes to the log, and try to add a "(see log)" for the second one.

No matter how important performance might seem to somebody tuning the code, it is NOT the only thing relevant to somebody running a simulation, and it cannot dominate all output. In the present form, we will write double notes using six lines of output for every single run where a user chose not to use all cores in a system.

IMHO, that is no longer friendly help, but overloading the user with non-critical information.

#5 Updated by Gerrit Code Review Bot almost 2 years ago

Gerrit received a related patchset '1' for Issue #2342.
Uploader: Berk Hess ()
Change-Id: gromacs~release-2018~I1fa8374d70891c3bc1caf2827210875e1c1a020f
Gerrit URL: https://gerrit.gromacs.org/7328

#6 Updated by Mark Abraham almost 2 years ago

  • Related to Bug #2345: Avoid duplicate over-subscription note+warning added

#7 Updated by Berk Hess almost 2 years ago

  • Category set to mdrun
  • Status changed from New to Fix uploaded
  • Assignee set to Berk Hess
  • Target version set to 2018-beta2

#8 Updated by Mark Abraham almost 2 years ago

  • Related to Bug #2346: inconsistent behavior of mdrun -nt 1 with mdrun -nt 3 when both are undersubscribed added

#9 Updated by Mark Abraham almost 2 years ago

  • Category deleted (mdrun)
  • Assignee deleted (Berk Hess)
  • Target version deleted (2018-beta2)

I noticed #2345 and #2346 while testing this issue and Berk's fix

#10 Updated by Berk Hess almost 2 years ago

  • Category set to mdrun
  • Status changed from Fix uploaded to Resolved
  • Assignee set to Berk Hess
  • Target version set to 2018-beta3
  • Affected version changed from 2018-beta1 to 2018

#12 Updated by Erik Lindahl almost 2 years ago

  • Status changed from Resolved to Closed

Thanks, Berk!

Also available in: Atom PDF