Change condition to use doVerticalTrace #211
Conversation
|
Testing summary |
RayTrace.cc
Outdated
|
|
||
| //special case for pesky near-vertical rays | ||
| if(dist<=requiredAccuracy){ | ||
| if(z_dist > 0. && xy_dist/z_dist <= tanThetaThreshold){ |
There was a problem hiding this comment.
Can we get some parentheses in this line? My anxiety is on me about the order in which the >, &&, and <= will be evaluated in this expression.
| // both cout | ||
| //std::cout << "Looking for estimate" << std::endl; | ||
| indexOfRefractionModel::RayEstimate est=rModel->estimateRayAngle(sourcePos.GetZ(), targetPos.GetZ(), dist); //TODO: put this back!!! | ||
| indexOfRefractionModel::RayEstimate est=rModel->estimateRayAngle(sourcePos.GetZ(), targetPos.GetZ(), xy_dist); //TODO: put this back!!! |
There was a problem hiding this comment.
You caught all mentions of dist that need to be updated right?
There was a problem hiding this comment.
Ya, it's a local variable, so it wouldn't compile if we missed any.
RayTrace.cc
Outdated
| bool fullyContained=(sourcePos.GetZ()<=0.0 && targetPos.GetZ()<=0.0); //whether the ray is entirely inside the ice | ||
| double dist = sqrt((targetPos.GetX()-sourcePos.GetX())*(targetPos.GetX()-sourcePos.GetX())+(targetPos.GetY()-sourcePos.GetY())*(targetPos.GetY()-sourcePos.GetY())); | ||
| rayTargetRecord target(targetPos.GetZ(),dist); | ||
| double xy_dist = sqrt((targetPos.GetX()-sourcePos.GetX())*(targetPos.GetX()-sourcePos.GetX())+(targetPos.GetY()-sourcePos.GetY())*(targetPos.GetY()-sourcePos.GetY())); |
There was a problem hiding this comment.
Since I'm already requesting changes, can you split this into a couple of lines so it's easier to follow the parentheses?
There was a problem hiding this comment.
Let me know if that's better. I hate how this was all written lol
This PR changes the condition to use the ray tracer aimed at near-vertical solutions,
doVerticalTrace.Previously, this function was used when the x-y distance between the target and source was smaller than the
requiredAccuracy(usually 0.2 meters). This condition was independent of the z-coordinate, so that even very nearly vertical sources (eg 0.7 m in x-y but 1.5 km in z) did not satisfy the condition, resulting in very slow ray tracing usingdoTrace.We've updated this to use
doVerticalTracewhenever the angle from the target's z-axis to the source is within 2 degrees of vertical. This should be more robust in terms of performance without affecting the ray tracing accuracy.