-
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
Bpm errors #538
base: master
Are you sure you want to change the base?
Bpm errors #538
Conversation
Hello @swhite2401 This looks very nice! I have no comment on the global design:
May be one could look at a way to look at the effect of specific kinds of errors: I take the case where all kinds of errors are assigned once for ever (but not enabled). One could want to look at:
Otherwise I have a few comments concerning implementation details, that can wait… |
Hello @lfarv, thanks for the feedback. If you agree with the design I will extend this as you suggest to provide more flexibility. I agree the implementation needs to be improved, we can iterate on this once the desired functionalities are all implemented. |
@swhite2401 : Perfect! |
@lfarv I have pushed my latest modifications (not tested), it is all yours! |
I have some proposals to add to the error assignment function:
Given the specificity of magnet errors, it could be convenient to separate Finally, I was also thinking of a "noise" BPM error, varying on each BPM crossing. This would make sense for |
I do have some concern about the relative errors though...I think the simplest would be to add another field I do not understand what you mean by separating magnet and BPM errors, isn't this already the case? Or you would like to produce to separate decorators? Besides that all the rest is fine for me! BPM noise could be interesting in fact! We may simply generate/apply them in the function |
There is at the moment a single |
9f24cc1
to
435ec73
Compare
Some of the modifications discussed above are implemented:
On the other hand, a single The documentation is ready. |
@lfarv, have you thought about this? An alternative could be to provide a relative option for |
@swhite2401 : Still there ?
This was exactly my initial idea, but you convinced me that another attribute would be simpler than an option (we can add both attributes). My idea is to provide a multipole description directly coming out of the magnetic measurements, normalised to the main component. This way, you have a single description for mechanically identical magnets, the multipoles just scale with the main strength. This is perfectly valid in the linear range. At saturation (which is unfortunately the usual case nowadays), it is approximate, but anyway better than a fixed value if you intend to vary the strength around to nominal working point. This seems to me the simplest way to describe systematic multipole errors, and is also probably useful for random errors: errors related to mechanical random errors also roughly scale with the current. By using a different attribute ("ScalingPolynomAErr", or better if you have ideas), I just comes as an addition to the present code, and can be implemented now or later: no fundamental change is needed. |
Hello @lfarv, yes still around for a couple days. Or course in this case if you change the main strength the error remains fixed, but my feeling is that for error studies we look at static lattices in general. I agree this can be added at any time...up to you! |
I case we would like a 'dynamic' behavior it think we would need to go for this one right? |
Still not clear for me… I'll detail the way I see it:
The point is that this make very easy to convert the results of magnetic measurements into an error attribute. Even errors on gradient integrals are easily described: you put 0.001 in the random ScalingPolynomBErr[1], and that's it! This leaves the "static" PolynomBErr acting as you defined it, it's just a new feature. And we may add more… By the way, we could add a |
Ok it is now all clear and I agree with what your propose, this is indeed very simple! Why should |
Then the question on multipoles remain... the scaling applies to all non-zero orders in |
You are right, no need!
The scaling would apply to all orders: you can include a spurious dipole component (systematic or random) in a quadrupole. For systematic errors, you should set 0 in the "main" component (it is already the PolynomB value), but you may set it for random errors (see above). The errors would scale with
This can be done easily by adding a "Strength" or "IntegratedStrength" property to the various elements ("DefaultOrder" is already defined). Assigning a "ScalingPolynomErr" to a One could normalise to the integrated strength (bending angle, K.l, H.l, etc.). Do you have a preference ? Integrated could apply to "thin" magnets, but there are no For "DQ"s (described as |
No real preference, integrated strength potentially offers some advantages as you mentioned and is generally what you get from magnetic measurements. |
So let's go for integrated strengths! |
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.
ready for me
What's new in the last commit:
|
This proposal introduces a possible solution to handle BPM, field and alignment errors.
Errors are added to an element with additional arguments
BPM*
for BPM errors and*Err
forPolynomA/B
and alignment errors. For BPM errors, it is only possible to modify the output offind_orbit
. For field and alignment errors one can either use a wrapper for selected functions (lattice_pass
,find_orbit
andget_optics
for now) or generate a new ring. In both cases the*Err
attribute is added to the standard existing attribute (For examplePolynomB = PolynomB + PolynomBErr
).Function provided:
-wrapper are provided for the functions
lattices_pass
,find_orbit
andget_optics
. This does not modify the input ring-associated new functions
*_err
are defined with the modified outputs-a function
enable_errors
is provided to return a copy of the lattice with errors included in existing attributes: this lattice can be used with any existingat
function, this new ring is protected by a private flag to avoid that the errors are applied twice-a function is provided to fetch the mean and std error of selected keys/attributes
Since this is a proposal I leave as draft for now, any suggestions?