Problem 557 - incorrect use of abs() instead of fabs()
Summary: incorrect use of abs() instead of fabs()
Status: RESOLVED INVALID
Alias: None
Product: Geant4
Classification: Unclassified
Component: event (show other problems)
Version: 5.2
Hardware: PC Linux
: P1 major
Assignee: Makoto.Asai
URL:
Depends on:
Blocks:
 
Reported: 2003-12-17 13:25 CET by torun
Modified: 2004-01-10 08:18 CET (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this problem.
Description torun 2003-12-17 13:25:45 CET
I was using precompiled 5.2.p01 release libraries for RedHat7.3-gcc3.2
downloaded from the geant4 website on a RedHat7.3 system but I suspect this
applies to many other releases as well.

While investigating bogus warning messages that looked like

  G4ParticleGun::pi+
  KineticEnergy and Momentum could be inconsistent
 (Momentum:0.291109 GeV/c  Mass:0.13957 GeV/c/c)
  KineticEnergy is overwritten!! 0.182452->0.183268GeV

from void G4ParticleGun::SetParticleMomentum(G4ParticleMomentum)

(which you can reproduce by adding a call to this function anywhere in user
code), I tracked down the problem to the following piece of code

    G4double mass = particle_definition->GetPDGMass();
    G4double p = aMomentum.mag();
    particle_momentum_direction = aMomentum.unit();
    if ((particle_energy>0.0)
      &&(abs(particle_energy+mass-sqrt(p*p+mass*mass))>keV))
    {
      ...
      G4cout << "  KineticEnergy and Momentum could be inconsistent"
      ...
    }

In general, when you write abs(x) where x is a double, what you get
depends on various factors: the expression it's used in, the operating
system, compiler and version, whether you '#include <cmath>' or are
'using namespace std', phase of the moon, etc. In the code snippet above, the
abs() silently truncates its double argument to an integer. I think it's always
better to use fabs in these cases to avoid confusion although one could probably
find some magic combination of compiler switches, etc. to get the right
behavior. Here, it results in a harmless warning but to check where else this
might be a real problem, I ran

  cd $G4INSTALL/source
  find . -regex '.*\.i?cc' -print | xargs grep -l '[^f]abs('

which returns a total of 171 hits on the 6.0 source tree (I am not including the
output since you can run the command to find out). I looked quickly through
about half the files in the list and 3/4 of them seemed to have the same misuse
of abs() instead of fabs(). I don't have time to enter a hundred bug reports but
somebody should go through the list and have them fixed, the files are
distributed in many components (geometry, physics, etc.).

It's disappointing to see such basic mistakes creep into the project
and go through whatever quality control/testing mechanisms there are
(this would surely have generated a compiler warning in most places and various
distributions in physics code would come out wrong in simple tests).
Comment 1 Gabriele Cosmo 2004-01-10 08:18:59 CET
The analysis reported by the user is NOT correct.
Geant4 makes consistent use of the inline template function abs(T) defined
in CLHEP, which is introduced with the inclusion of the utility header
"global.hh" provided in Geant4. Therefore, the compiler should provide the
correct template specialization, according to the type given as argument.
This workaround was put in place long time ago right to overcome the
ambiguity caused by the inclusion of non standard headers from C and the
unavailability of <cmath> on some compilers.
Note that in C++, according to the standard, abs() is overloaded for different
data types and can be used for all data types (integral or floating-point).
In future, when the major numeric functions will be defined in the 'std'
namespace for all the compilers we support, all calls should migrate to
std::abs(), which should be the -correct- way to handle numeric functions
in C++.
Moreover, using the G4 build setup, a compilation warning is thrown (especially
with the gcc compiler), if abs(int) is used for 'double'. This makes easy to
detect if the wrong function is picked up.
The user should verify his build environment is not invalidating the default
system and Geant4 setups.