[mini] move position when step size advance fails #450
Conversation
|
Can one of the admins verify this patch? |
|
CI failed in reproducibility in sync AdePT, a longer standing and rare issue. Just restarted it. |
| if (!continueIteration) { | ||
| // Let's move to the other side of this boundary -- this side we cannot progress !! | ||
| move = Navigator_t::kBoundaryPush; // curvedStep | ||
| position += move * chordDir; |
There was a problem hiding this comment.
I am not quite sure about this: it introduces a semantics change after crossing the boundary; instead of leaving the point on the boundary, it will slightly push it, which may have adverse effects in case of back-scattering. On the other hand, you are right that the stepDone is updated with the move, which makes it inconsistent. I wonder if we should rather not readjust stepDone with this tiny move.
There was a problem hiding this comment.
In case of back scattering, it should never reach that part because of this:
if (move <= kDistCheckPush) {
#if ADEPT_DEBUG_TRACK > 0
if (verbose) {
printf("| BACK-SCATTERING or WRONG RELOCATION detected hitting ");
next_state.Print();
}
#endif
zero_first_step = true;
return 0.;But of course I am open to just delete the entire block with
if (!continueIteration) {because if it doesn't update the position, it doesn't do anything useful. Then we don't even need the continueIteration
either anymore... So definitely in the current state it doesn't do anything useful, we should see how to resolve it, this would be only a quick fix to make it consistent, not to address the whole problem.
It is not super urgent, it doesn't fix stuck tracks, but this should be reviewed and fixed
3 participants
This mini PR fixes a bug reported by code analysis from @dkonst13:
The the loop of reducing the step size when stuck on the boundary, the
movewas adjusted, but the position was not changed. As themoveaffects the remains and the stepDone, the position must be also adjusted here, otherwise it is not consistent.