0000240: abs() method used on Real values in core::scoring

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

Project

Category

View Status

Date Submitted

Last Update

0000240

Rosetta

[All Projects] Incorrect Results

public

2013-04-02 16:21

2013-04-03 12:29

Reporter

Labonte

Assigned To

pbradley

Priority

normal

Severity

major

Reproducibility

always

Status

resolved

Resolution

fixed

Platform

All platforms

Any

OS Version

Any

Product Version

Trunk

Fixed in Version

Trunk

Summary

0000240: abs() method used on Real values in core::scoring

Description

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!

Additional Information

http://www.cplusplus.com/reference/cstdlib/abs/ [^]

Tags

No tags attached.

Application(s) Affected

DNA & RNA applications?

Command Line Used

N/A

Developer Options

Fixed in SVN Version

54660

Attached Files

Relationships

Notes
(0000242) rmoretti (Attentive Developer) 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 (Developer) 2013-04-02 17:37	Thanks, Rocco!

(0000247) pbradley (Developer) 2013-04-03 12:29	Thanks for the tip! Switched to std::fabs and included <cmath> in revision 54660

Issue History
Date Modified	Username	Field	Change
2013-04-02 16:21	Labonte	New Issue
2013-04-02 16:41	rmoretti	Note Added: 0000242
2013-04-02 17:37	Labonte	Note Added: 0000244
2013-04-02 17:38	Labonte	Description Updated	View Revisions
2013-04-02 17:39	Labonte	Priority	low => normal
2013-04-02 17:39	Labonte	Severity	minor => major
2013-04-03 09:32	delucasl	Assigned To	=> pbradley
2013-04-03 09:32	delucasl	Status	new => assigned
2013-04-03 12:29	pbradley	Fixed in SVN Version	=> 54660
2013-04-03 12:29	pbradley	Note Added: 0000247
2013-04-03 12:29	pbradley	Status	assigned => resolved
2013-04-03 12:29	pbradley	Resolution	open => fixed
2013-04-03 12:29	pbradley	Fixed in Version	=> Trunk