-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(gx2f): constrain update to initial volume #3411
Conversation
📊: Physics performance monitoring for 03d46cbphysmon summary
|
do you know why we end up updating the parameters so drastically @AJPfleger ? potentially we can also damp the learning rate or clamp directly on the delta param vector? |
I would look into damping/accelerating, after I get the material in. Maybe that already improves it. |
Coverage is failing. I guess I could build a detector that has volumes A and B in it. Then create measurements, starting the propagation from volume A. Then manually choose a starting position in volume B. Is it worth the effort? |
|
During the Athena integration, I ran again into the issue described in
pathCorrection=inf
on cylinderSurface #3267This PR does not fix the navigation itself but avoids the GX2F ending up in a state, where the above issue would occur. I investigated different approaches to avoid ending up in a situation, where an FPE would occur.
Simulate division
The FPE occurs, when we are calling the path correction. Checking and aborting if the x-y components for position and direction are zero would be sufficient. However, we should also apply the transformation to the position first, which seemed a bit difficult in the actor.
Using this approach too many tracks were filtered out.
I didn't not check, if all of them would fail anyhow, though.
Constrain parameters (Used)
I had the issue only, when starting from a volume, that was different from the initial volume of the starting parameters. However, there were also cases, where I switched volumes and didn't crash. After looking into all non-crashing cases (analysed 13 tracks, because then the Athena truth tracking crashed), I saw, that all of them would later end with the error `NotEnoughMeasurements. Therefore, we effectively do not lose any tracks but detect their failure earlier. In the physmon, we also see with larger statistics no change to the current behaviour.
Testing
I built a new detector for testing this behaviour. I tried to reuse the old one by adding extra parameters, but I didn't succeed. Somehow, I had to flip the detector to force the volume change.
I also added a visualisation script to it, that generates an
.obj
of the detector and draws the measurements. This one I used for the development since the beginning but didn't find a proper place to put it in. The new detector might be complex enough, that we maybe want to visualise it during development.Blocked