-
Notifications
You must be signed in to change notification settings - Fork 33
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
Variables and Observables #603
Conversation
This will take a long time to review!! I can already tell you that the modification of the matching is fine for me, we should keep the code a simple as possible and this is a clear improvement. |
@swhite2401: Well, there is no pressure at the moment. I have no development waiting for that. If it stays for long as a separate branch while you look at it, I will rebase it on master at each significant upgrade, so that it's does not get too late with respect to master. If you agree, you could close the issue #603 and try to keep the list short! @simoneliuzzo: I do not expect you to spend time on reviewing python code, but since this was a burning topic at the meeting on errors and corrections, it would be nice if you can just have a look at the global idea here. Though it's not implemented yet, a BpmObservable would be a clean way to introduce BPM errors, for instance. |
I closed the issue and will go through it slowly. Concerning errors and correction, the question is not as hot as it used to be since there is a pySC coming online that handles all of this and @simoneliuzzo also has something that is working very well. I still think that we should integrate something in AT but I am also not satisfied with the present proposal, this is not what we would have liked. I have been too busy to look into alternatives.... but I will at some point. Since there is no emergency, I would prefer that we evaluate all options before merging anything. For the moment the main application that I can see would be control system interface, there is more and more interest in having a 'python Middle Layer' and I think these features would be extremely useful to implement it! |
Nothing here concerning errors and corrections, apart that Observables (modified if needed) should be the targets of corrections (as they are for matching). |
Dear @lfarv and @swhite2401 , thank you for these developments. Is the matching faster? Looking at the test code, it is not shorter to write a matching, but it looks more easy to read. I doubt this was the real target of the development. I think the existing matching should be kept working, independently, for backward compatibility. I use rather often geometry/survey matching. Something that could be GeometryObservable or SurveyObservable. How easy is it to write a private ******Observable? How easy is it to specify a user defined function as variable? For example, change 5 dipoles keeping: 1) the sum constant, 2) the ratio between the fields in dipole 1 and dipole 3 constant, 3) assigning a quadrupolar gradient to each of the 5 dipoles proportional to the dipole field. (this is done in PETRAIV matching, very easy to write in MADX/mad8, rather cumbersome in AT / pyAT). The pySC development is going on for the moment simply translating the matlab SC. I will point T.Hellert and L.Malina to this pull request since they are doing the work and know the code. Also from my side, I may not promise that I will soon look at these features. thank you best regards |
Hello @simoneliuzzo, To answer your first remark: no, matching is not faster. It is identical to the present version, It has just been adapted for the new Then you raise two very interesting questions: New VariablesThe present base class Variables are evaluated in order, so the custom variable can use the values set by the 2 previous ones. This just means writing a python class providing I’ll propose a similar (but simpler) example in the documentation to illustrate the use of custom Now to compare with MADX:
New ObservablesSimilarly, a custom evaluation function can compute anything, like computing the lattice geometry and extracting the desired values. Simply writing a python evaluation function is the simplest way of creating new observables based on linear optics, for instance. However, for performance reasons, the idea is to separate the computation itself, done once (call So thanks Simone for your input, |
I added more documentation on variables, including an example of custom variables (to answer one of @simoneliuzzo's questions). |
There is now a full notebook with a matching example using a custom variable |
Added another example notebook showing how to use various observables. |
This notebook in the documentation is very nice! @lcarver, do you think you could do the same thing with the collective effects examples?? |
I think most examples should be set as notebooks. In addition, notebooks are executed by Sphinx when generating the doc (by default, otherwise one can still store "pre-computed" notebooks), so we are sure that the outputs are correct ! To get the exact appearance of the notebook, it's better to have Sphinx installed, since The next improvement would be to add a link somewhere allowing users to download the notebooks. |
Dear @lfarv, below an example of something I am unable to do (if not in a cumbersome way) in AT, but was possible in mad8 dipoles in standard cell
dipoles in dispersion suppressor
The variable
In this
I think this should be the target of updated matching tools for AT. Not to mention, the matching process takes a negligible time. |
@simoneliuzzo , I think you have to think of you ring as an object and provide a function that rebuilds it from the variables. Each time you change a variable you would then call this function to rebuild the whole ring. I havre done something similar for EBS, I'll propose something for your example. This is certainly not a few lines because you need to write the code that builds your ring (the same as MADX) but once this is done it is relatively simple. Concerning the speed of the matching, I have done test and AT is roughly equivalent: for your information MADX by default matches uncoupled lattice, this makes a big deference. To activate this in AT you may use |
Thanks @simoneliuzzo for the example. I have another option which could avoid rebuilding the lattice. I agree with @swhite2401 that a function to build the lattice is a good thing, it allows building blocks and combine them. It's also good for efficiency to reuse the same element when it appears several times in the lattice, rather than creating many identical elements. Then, modifying the single element is faster that modifying the many identical ones. But rebuilding the lattice takes time, and during a matching it costs. To avoid that I introduced 2 new classes: a an In @simoneliuzzo's example, this could give:
the length of the DL4A would be set to Then, letting elements be varied is done like this: exp4 = Expression(dl4a_pts, 'PolynomB', '{1} * {2} * (1+{0})', DL12, DL_LEN, BXFR, index = 1)
exp3 = Expression(dl3a_pts, 'PolynomB', '{1} * {2} * (2-{0})', DL12, DL_LEN, BXFR, index = 1)
exp2 = Expression(dl2a_pts, 'PolynomB', '{1} * {2} * (2-{0})', DL12, DL_LEN, BXFR, index = 1)
exp1 = Expression(dl1a_pts, 'PolynomB', '{1} * {2} * (1+{0})', DL12, DL_LEN, BXFR, index = 1)
... The DL12 Parameter is given to the matching, and its variation will be forwarded to all elements. For now, both Parameter and Expression are working, the exact interface and the synchronisation are still to be done. |
The example notebook is modified to demonstrate the 2 methods for handling correlated variables:
|
Hello @lfarv ,
Now concerning deferred variables in general, the whole idea is really to defer the evaluation of these expression only when they are needed, i.e. when entering the tracking engine or reading the attribute. In theory you may want to assign these variables or expressions to any lattice element attribute. Finally, this PR is becoming too huge and keeps growing. This was already a comment when you first proposed it. We need to split it otherwise it will never be merged. It would like to propose the following PRs:
|
Hello @swhite2401
No good reason for this: I proposed that as the easiest way, but any better idea is welcome. I just took @simoneliuzzo' s 1st example: dipole length =
Well, it depends. I agree that parametrising the lattice elements would be a major improvement. However, concerning matching, the constraints you want to impose depend a lot on what you want to match. In the example notebook, I took the example of moving monitor in a straight section. The constraint l1 + l2 = constant is related to this matching case, but may be irrelevant for matching something else. This kind of constraint is not part of any element an has no reason to be part of the lattice description. Also, parametrising lattice elements is a major task, out of the scope of this PR. In particular, the evaluation time is delicate: for sure not at tracking time (it would be evaluated at each pass through the element), preferably not at the beginning of tracking (if you track turn by turn, it will be evaluated on each turn), but only when a parameter is varied, more precisely after all parameters have been varied. The elements should stay as they are (also to avoid modifying the integrators), and parametrising should be done through additional attributes: it's a big work! So the approach in this PR is much simpler: all computations are done after all variables in a VariableList are set. Finally I agree with you scheduling, with this remark: |
Ok, I can propose something, can I add functions to this branch or should I make my own copy? I still think that all of these belong to the lattice! These are basically lattice parametrizations and if you take time to define them you would like to save them altogether with your lattice (and I know this is very complex...). Your proposal is obviously much simpler, but we should be ambitious, I know for a fact that these deferred variables is what is stopping a lot of people form using anything else than MADX. Having such functionality in AT would be a huge asset. But clearly out of the scope of the PR I agree and this is why I would like to split things a bit. I fully agree that we should not have 2 matching modules, this is confusing and requires a lot work to maintain, so you are probably right the first 2 steps have to merged in one go. But this is just my personal opinion... maybe we could make a new release first? |
Sure, feel free to commit to this branch
Could be, but then you have to modify the lattice to add the correlations required by such or such matching request. Why not… And wait for the full parametrisation for correlated matching. Unless we start with a simpler solution, which is not incompatible with the full one.
I have the same opinion. But then we have to assume the compatibility break!
Sure, I'm preparing the pull request and updating the developer documentation of the release procedure. There is still a pending PR fixing the format of geometry output, with again a problem of compatibility… |
See #696 |
This PR introduces some classes used for matching and response matrix computation, but which may be used also for other purposes (plotting…).
Basically, Observables are anything which can be looked at in a lattice (element attributes, optics data…), possibly while varying Variables (anything that can be tuned). They are subdivided in TrajectoryObervable, OrbitObservable, LocalOpticsObservable, GlobalOpticsObservable, EmittanceObservable, LatticeObservable, each resulting from a different computation. The idea is to propose a single interface for these different things. Of course new ones can be added.
For evaluation, Observables must be grouped in an ObservableList so that the necessary computations are optimised.
Similarly, Variables may be grouped in a VariableList for matching.
The documentation for this new
latticetools
package is here (updated on each commit in this branch).Though perfectly usable from now on, this is a work in progress: suggestion are welcome for modifications, additions, documentation…
Warning: as it is now, the new classes are conflicting with class definitions in the
matching.py
module. This is because I prefered to keep the "Variable" and "ElementVariable" class names to refer to the new objects. I modified thematch()
function in thematching.py
module to use the new class (it's much simpler now!).If one wants to keep the existing module, one should find another name for the two conflicting classes, and also keep two different matching functions (not very nice, but can be done). To be discussed.