## Bug #25559

### geo::WireGeo wire angle calculation

Status:
Closed
Priority:
Normal
Category:
Geometry
Target version:
-
Start date:
02/25/2021
Due date:
% Done:

100%

Estimated time:
Occurs In:
Experiment:
DUNE
Co-Assignees:
Duration:

Description

I have encountered an issue with the wire angle returned by WireGeo::ThetaZ() for horizontal wires (i.e., ThetaZ=0) while implementing a sorting wire algorithm for an instance of GeoObjectSorter. The value of ThetaZ appears to be NaN for some wires. In my sortWire method I put a print of the type:

if( w1.ThetaZ()!=w1.ThetaZ() ){
cout<<"w1 " << w1.WireInfo("  ", 3)<<" half length " << w1.HalfL()<<endl;
}

and this is an example for a problematic wire of the above print:
w1 wire from (325.03,-500.827,148.996) to (325.03,-500.827,297.892) (148.896 cm long), theta(z)=nan rad
center at (325.03,-500.827,223.444) cm half length 74.448

From a quick look at the constructor of geo::WireGeo, the wire angle is calculated as

fThetaZ = std::acos((end.Z() - fCenter.Z())/fHalfL);

A likely culprit is the rounding errors in the argument of acos that causing it to go just above 1. Even if this is in a some insignificant decimal place it can still cause a NaN for the return.

Could one add something like an std::clamp function to limit range of the argument to +/-1 or some similar fix to avoid such a rounding issue popping up?

### History

#### #1 Updated by Gianluca Petrilloabout 2 months ago

I feel your analysis is right on the spot... although in ICARUS none of the 8416 horizontal wires ever showed that feat (it probably depends also on the amount of rotations needed to get there...).
Anyway, I have put in a larcorealg pull request #15 on the topic.
You can pull in feature/gp_issue25559 from my fork of larcorealg to see if that is solving the issue.

#### #2 Updated by Vyacheslav Galymovabout 2 months ago

Yes, I can confirm this solved the issue.

#### #3 Updated by Kyle Knoepfelabout 2 months ago

• Assignee set to Gianluca Petrillo
• Status changed from New to Assigned

Thanks, Gianluca. We will look at the PR and approve as appropriate.

#### #4 Updated by Lynn Garrenabout 2 months ago

larcorealg PR 15 is included in larsoft v09_17_02.

#### #5 Updated by Kyle Knoepfelabout 2 months ago

• % Done changed from 0 to 100
• Status changed from Assigned to Closed

Also available in: Atom PDF