MantisBT - Rosetta
View Issue Details
0000240Rosetta[All Projects] Incorrect Resultspublic2013-04-02 16:212013-04-03 12:29
Labonte 
pbradley 
normalmajoralways
resolvedfixed 
All platformsAnyAny
Trunk 
Trunk 
DNA & RNA applications?
N/A
54660
0000240: abs() method used on Real values in core::scoring
This may not be an error, but I'd like someone who knows more to double check it, just in case, because it looks very wrong to me and is throwing a ton of warnings.

In core/scoring/dna/base_geometry.cc, a lot of asserts are using the int abs(int) method but passing Reals. Rather than just cast everything to an int, I wanted to make sure that this is the intended behavior, because there is also a float abs(float) (in a different namespace, cmath) that could be used.

For an example, see l. 1246:

assert( abs( params[1] - asin( dot( MBT.col_z(), cross( M2.col_y(), M1.col_y() ) ) ) ) + abs( params[1] - asin( dot( MBT.col_z(), cross( M2.col_x(), M1.col_x() ) ) ) ) < 1e-2 );

where params is a vector1<Real>.

Not all of the issues are in asserts either -- l. 1303:

Real dot = std::abs( n12.dot( ( atoms[4]-atoms[3] ).normalized() ) );

where n12 is an xyzVector. This looks very bad to me; or am I missing something?

No one listed him- or herself as author on this file, but whoever you are, could you fix all the abs() calls in this file, whether by casting or by using cmath::abs()? Thanks!
http://www.cplusplus.com/reference/cstdlib/abs/ [^]
No tags attached.
Issue History
2013-04-02 16:21LabonteNew Issue
2013-04-02 16:41rmorettiNote Added: 0000242
2013-04-02 17:37LabonteNote Added: 0000244
2013-04-02 17:38LabonteDescription Updatedbug_revision_view_page.php?rev_id=71#r71
2013-04-02 17:39LabontePrioritylow => normal
2013-04-02 17:39LabonteSeverityminor => major
2013-04-03 09:32delucaslAssigned To => pbradley
2013-04-03 09:32delucaslStatusnew => assigned
2013-04-03 12:29pbradleyFixed in SVN Version => 54660
2013-04-03 12:29pbradleyNote Added: 0000247
2013-04-03 12:29pbradleyStatusassigned => resolved
2013-04-03 12:29pbradleyResolutionopen => fixed
2013-04-03 12:29pbradleyFixed in Version => Trunk

Notes
(0000242)
rmoretti   
2013-04-02 16:41   
Note: For those who have trouble finding it, the file is core/scoring/dna/base_geometry.cc

My guess would be this is a potential bug due to unawaredness that the abs function doesn't have sane polymophism.

svn praise (aka svn blame) and the svn server places most to the initial commits of the code by Phil Bradley some 6 years ago. Given that he's still actively working on code that likely uses these functions, I'd recommend shooting him an email directly.
(0000244)
Labonte   
2013-04-02 17:37   
Thanks, Rocco!
(0000247)
pbradley   
2013-04-03 12:29   
Thanks for the tip! Switched to std::fabs and included <cmath> in revision 54660