Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Use new diff() function for absolute difference between two unsigned coords#64

Open
jmarshall wants to merge 1 commit intogenome:masterfrom
jmarshall:rewrite-abs
Open

Use new diff() function for absolute difference between two unsigned coords#64
jmarshall wants to merge 1 commit intogenome:masterfrom
jmarshall:rewrite-abs

Conversation

@jmarshall
Copy link
Contributor

@jmarshall jmarshall commented May 23, 2017

Fixes the ambiguous abs() compilation errors described in #62. Casting to int is not really the best way to fix this, as the cause of the problem is that the arguments are unsigned int rather than int and what's really needed is some sort of abs_difference function that works on unsigneds natively.

There are several other source files that would also produce these compiler errors, but those source files happen to also #include <math.h> which gives them float abs(float)/etc functions that resolve the ambiguous function choice while probably making things worse.

This PR adds a new unsigned diff(unsigned, unsigned) function and uses it instead of raw abs() in all these source files. It also adds in pindel.h an implementation of this function that computes the difference by cases rather than by using abs(), so as to avoid overflow — which would occur for abs()-based methods when comparing a coordinate near 0 with one near the end of a 232-base chromosome. (If this is overkill, the implementation could be replaced with an abs()-based one, but I doubt there is any difference in efficiency.)

Using abs(pos1-pos2) has potential to overflow and, when pos1/pos2 are
unsigned, fails to compile when there are ambiguous signed int/signed
long/etc std::abs() functions to choose between.  Avoid these issues
by introducing a function that does what is really needed, namely
finding the difference without sign between the two coordinates.
@jdemeul
Copy link

jdemeul commented Aug 9, 2018

Ran into the ambiguous abs() compilation errors described in #62 as well.
Fixed by building this PR, thanks @jmarshall!

exic pushed a commit to exic/pindel that referenced this pull request Jul 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants