Project

General

Profile

Task #2249

Clarify rounding of SIMD round

Added by Roland Schulz about 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Difficulty:
uncategorized
Close

Description

Adding

GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(3), round(setSimdRealFrom1R(2.5)));
to SimdFloatingpointTest,round fails on AVX. The doxygen of round should specify what happens with ties (to zero (like c++), to even (like IEEE), depending on rounding mode set by user (like current AVX implementation), or undefined). And there should be a test which checks for ties (unless undefined).

Associated revisions

Revision 41531c0f (diff)
Added by Roland Schulz almost 2 years ago

Added doxygen note to clarify rounding mode of SIMD

Also moved unit test of rounding mode into own test to make
obvious that rounding mode is independent of ldexp.

Fixes #2249

Change-Id: Iae66e080de141ace7f2b1298f94fdb867f8632fe

History

#1 Updated by Erik Lindahl almost 2 years ago

I don't think we can (or at least want to) require a specific rounding mode.

While the vast majority uses IEEE, there have been some platforms (IBM Power with VMX or VSX SIMD) that used non-standard rounding either for round() or the float-to-integer-conversions. For this reason (and our exp() implementations, the only thing we require is that those to two rounding functionalities use the same rounding mode - which will make sure the algorithms produce correct results.

The main concern is that the float-to-integer conversion is needed for tables in the most performance-sensitive loop of the entire program, and requiring any rounding mode that isn't natively supported on a specific platform could lead to a very large performance hit.

#2 Updated by Mark Abraham almost 2 years ago

Sounds like we should test that those conversions are consistent, and document that the behaviour of our round() wrapper (and perhaps the conversions?) is implementation-defined.

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

Gerrit received a related patchset '1' for Issue #2249.
Uploader: Roland Schulz ()
Change-Id: gromacs~release-2018~Iae66e080de141ace7f2b1298f94fdb867f8632fe
Gerrit URL: https://gerrit.gromacs.org/7294

#4 Updated by Roland Schulz almost 2 years ago

  • Status changed from New to Fix uploaded

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

  • Target version set to 2018-beta3

Set target to beta3 as proposed implementation does not have approval yet.

#6 Updated by Roland Schulz almost 2 years ago

  • Status changed from Fix uploaded to Resolved

#7 Updated by Roland Schulz almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF