-
Notifications
You must be signed in to change notification settings - Fork 34
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
turn off quantdiff in ohmi_envelope #840
Conversation
The idea here is to allow users to continue working while preventing a wrong usage that is to call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the problem results from a more sever bug in random generators. I'll start looking at that.
The fix OK to avoid crashes. But removing quantum diffusion most likely removes all radiation effects, so the result is still probably irrelevant. It's indeed a wrong usage. Another possibility would be to test the passmethods and raise an error if there is any diffusion element.
Anyway, it's ok for me as it is, it's up to you…
Why would it remove all radiation effects? We still have the rad passes and correct calculation no? |
Ok for collective effects. But for radiation, it's either One could do the replacement automatically, but I'm reluctant to silently modify the user's choices… My preferred option is to warn the user that what he asks is wrong, and help him make the adequate corrections. |
Ah ok now I get it , in fact I had completely ingnore magnet quantdiff passes, and consisdered the QuantumDiffusion element only, but you are right we should cover all cases. I am nevertheless a bit reluctant to raise an error because ohm_envelope is called in many other functions to get the beam emittance and this may break other parts of the code... In this specific case I think that switching, or turning off passmethods is ok, the returned emittance value will be correct. I can make a decorator |
I don't see how an error if Raising an error has a pedagogic interest: it recalls, or teaches, the user that computing any optics parameter with random elements does not make sense ! Otherwise, he may repeat the mistake for ever… Practically, all optics functions are concerned. Many (including But I'm afraid that step by step we are slowing down AT. |
@swhite2401 : sorry, I was answering too fast ! In fact, all "4D" functions are already protected. So the problem only concerns Having a full protection, for an event which should very rarely happen is complicated and seems too penalising for me. I'm definitely in favour of the simplest approach: either a test test on pass methods and an error, or the option in this PR as it is (already approved): turn off all random elements. |
Ok I agree, then let's merge this one, and if we have complaints we can envisage something else. I merge as is? Any comments? |
OK for me |
Since #737
ohmi_envelope
exits with a segmentation fault when an element with a random number generation is present in the lattice (in the issue reported the QuantumDiffusion element).This PR propose a quick fix to prevent this issue, however, there seem to be a more profound bug with random generators that could affect other function.