Skip to content
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

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Bpm errors #538

wants to merge 28 commits into from

Conversation

swhite2401
Copy link
Contributor

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 for PolynomA/B and alignment errors. For BPM errors, it is only possible to modify the output of find_orbit. For field and alignment errors one can either use a wrapper for selected functions (lattice_pass, find_orbit and get_optics for now) or generate a new ring. In both cases the *Err attribute is added to the standard existing attribute (For example PolynomB = PolynomB + PolynomBErr).

Function provided:
-wrapper are provided for the functions lattices_pass, find_orbit and get_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 existing at 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?

@lfarv
Copy link
Contributor

lfarv commented Dec 11, 2022

Hello @swhite2401

This looks very nice! I have no comment on the global design:

  • the provided functions seem to fulfil most of the needs
  • they are simple enough and easy to use

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:

  • magnet errors without BPM errors: for this you can explicitly call enable_errors, and then use the "normal" optics functions which ignore BPM errors,
  • BPM errors only: for this you could explicitly set the _has_errors attribute on the initial lattice and call the *_err functions. Not very elegant, but possible,
  • Look independently at alignement, tilt or field errors. That would need a version of enable_errors with a selector, with a default of enabling everything. And go to bullet 1.

Otherwise I have a few comments concerning implementation details, that can wait…

@swhite2401
Copy link
Contributor Author

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.

@lfarv
Copy link
Contributor

lfarv commented Dec 12, 2022

@swhite2401 : Perfect!

@swhite2401
Copy link
Contributor Author

@lfarv I have pushed my latest modifications (not tested), it is all yours!

@lfarv
Copy link
Contributor

lfarv commented Dec 16, 2022

I have some proposals to add to the error assignment function:

  1. For all errors: It should be possible to specify both systematic errors (identical on all elements) and random errors (randomly distributed over elements). For this, we could decide that the "value" in keyword arguments could be either a tuple "(systematic, random)" or a single item, which would be attributed to random, as it is done now. Also, by making use of the broadcasting feature, the "systematic" value could set a completely predefined set of errors to all elements.
  2. For magnet field errors: it would be convenient to specify PolynomA/BErr either in absolute (added to PolynomB), or in relative (multiplied be the "strength" of the magnet before addition). That's the easiest way to introduce gradient integral random errors or systematic multipolar errors. The "strength" would be PolynomB[1] for quads, PolynomB[2] for sexts and so on. What about "Multipole" objects ? How to indicate the choice ? A "relative"bool keyword argument ?
  3. For gain, offset, shift errors: they are (2,) arrays, for H and V components. Do we allow a single value to indicate the same value in both planes ?

Given the specificity of magnet errors, it could be convenient to separate assign_bpm_errors and assign_magnet_errors. This would also have the advantage of making the documentation cleaner: it will be quite long. What do you think of that?

Finally, I was also thinking of a "noise" BPM error, varying on each BPM crossing. This would make sense for lattice_pass, possibly affecting the determination of tunes. But problematic for closed orbit: the orbit would change on each call… But why not? Let's keep this for later…

@swhite2401
Copy link
Contributor Author

I do have some concern about the relative errors though...I think the simplest would be to add another field Polynom[AB]Scale for instance that would just multiply the result after the offset was applied (default being one for all multipoles). I think this is more flexible and in case of calibration errors I believe all multipoles would scale no?
Similarly to other fields it is activated when it is present.

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 _apply_bpm_track_error. It would just require an additional sigma argument. and also separated decorator for each function producing an orbit, but you had in mind to separate them anyway right?

@lfarv
Copy link
Contributor

lfarv commented Dec 16, 2022

I do not understand what you mean by separating magnet and BPM errors, isn't this already the case?

There is at the moment a single assign_errors function for all kinds of errors. If we need keyword flags specific for BPMs or magnets, it could be cleaner to have rather assign_bpm_errors and assign_magnet_errors. The description of all errors will also be rather long, and could be shared between both functions. But if there is no specific feature, we keep is as it is!

@lfarv lfarv force-pushed the bpm_errors branch 2 times, most recently from 9f24cc1 to 435ec73 Compare December 19, 2022 14:33
@lfarv
Copy link
Contributor

lfarv commented Dec 19, 2022

Some of the modifications discussed above are implemented:

  • a tuple (systematic, random), or a single value for random only is implemented for all errors,
  • for offsets/shifts and gains, a [x, y] array defines H and V values, but a single number is accepted to set both planes to the same value.
  • for rotations, a single value sets tilt only, and a triplet sets [tilt, pitch, yaw].

On the other hand, a single assign_errors function is kept for all errors (no need to split), and the question on PolynomA/B is postponed until we find a satisfactory solution.

The documentation is ready.

@swhite2401
Copy link
Contributor Author

the question on PolynomA/B is postponed until we find a satisfactory solution.

@lfarv, have you thought about this? An alternative could be to provide a relative option for PolynomA/B in the assign_errors function?

@lfarv
Copy link
Contributor

lfarv commented Dec 21, 2022

@swhite2401 : Still there ?

An alternative could be to provide a relative option for PolynomA/B in the assign_errors function?

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.

@swhite2401
Copy link
Contributor Author

Hello @lfarv, yes still around for a couple days.
Well, I was thinking of some simpler: instead of providing absolute value to assign_errors you provide a percentage instead.
Then the absolute value is derived internally using PolynomA/B*error and assigned to PolynomBErr.

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!

@swhite2401
Copy link
Contributor Author

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.

I case we would like a 'dynamic' behavior it think we would need to go for this one right?

@lfarv
Copy link
Contributor

lfarv commented Dec 21, 2022

Still not clear for me… I'll detail the way I see it:

  1. The perfect lattice is defined with "standard" quadrupoles: PolynomB[1]=K, nothing else, neither in B nor in A,
  2. we take the normalised multipole analysis from the magnetic measurement of let's say type "A" quadrupoles: it contains octupole, dodecapole etc. We introduce this with assign_errors in the "ScalingPolynomBErr" attribute of all type "A" quadrupoles,
  3. we can play with this (still perfect) lattice: matching, tune optimisation...
  4. to look at the effect of errors, in enable_errors, we sum PolynomB, K*ScalingPolynomBErr and PolynomBErr (if any), store the sum in PolynomB to run say dynamic acceptance,
  5. We can make any new tuning on the initial lattice, no need to re-assign_errors, the magnetic contents of each magnet is frozen in the ScalingPolynomBErr attribute.

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 copy option in enable_errors ?

@swhite2401
Copy link
Contributor Author

Ok it is now all clear and I agree with what your propose, this is indeed very simple!

Why should copy be optional in enable_errors, you never want to modify ring in-place no?

@swhite2401
Copy link
Contributor Author

Then the question on multipoles remain... the scaling applies to all non-zero orders in PolynomB/A?

@lfarv
Copy link
Contributor

lfarv commented Dec 21, 2022

Why should copy be optional in enable_errors, you never want to modify ring in-place no?

You are right, no need!

Then the question on multipoles remain... the scaling applies to all non-zero orders in PolynomB/A?

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

  • 1/(bending radius) for Dipole [m-1]
  • PolynomB[1] for Quadrupole [m-2]
  • PolynomB[2] for Sextupole [m-3]
  • PolynomB[3] for Octupole [m-4]

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 Multipole should fail: there you must use fixed errors

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 ThinQuadrupole or so.

For "DQ"s (described as Dipole), it will be static whether you use PolynomBErr or ScalingPolynomBErr (unless the radius varies): it's supposed to be a fixed magnet.

@swhite2401
Copy link
Contributor Author

One could normalise to the integrated strength (bending angle, K.l, H.l, etc.). Do you have a preference ?

No real preference, integrated strength potentially offers some advantages as you mentioned and is generally what you get from magnetic measurements.

@lfarv
Copy link
Contributor

lfarv commented Dec 21, 2022

So let's go for integrated strengths!

@lfarv lfarv marked this pull request as ready for review December 30, 2022 14:42
Copy link
Contributor

@lfarv lfarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ready for me

@lfarv
Copy link
Contributor

lfarv commented Jan 23, 2023

What's new in the last commit:

  • the generation of random values is moved to enable_errors, with the following benefits:

    • no need to re-run assign_errors to generate a new error set: the errors are assigned once for ever,
    • easier to generate a reproducible set of errors: a single seed in enable_errors is needed rather than a seed in each of multiple assign_errors commands.

    For a given seed, the errors generated for an error category do not depend on the other enabled errors: for instance random multipole errors do not depend upon whether you enable alignment errors or not.

  • there are no more special *_err functions. But a new boolean monitor_errors keyword is added to lattice_pass, find_orbit and get_optics commands. It is necessary because in the absence of special "bpm_reading" output argument, these function must still give the real orbit for internal use (optics or other parameter computation), even when errors are enabled in a lattice. The BPM error contribution is added to the "closed orbit" output in the last step when a user asks for monitor errors. Otherwise, the apply_bpm_orbit_errors function is available to convert any closed orbit output into monitor readings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Python For python AT code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants