search

atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

Added KStep and HStep keyword arguments to fit_tune and fit_chrom #711

Closed simoneliuzzo closed 10 months ago

simoneliuzzo commented 10 months ago

use available scaling for gradient variation used by fit_tune and fit_chrom in at/pyat/matching/globalfit.py

simoneliuzzo commented 10 months ago

An other alternative could be to add the DK to the list in at.lattice.options.DConstants

lfarv commented 10 months ago

@simoneliuzzo : the index argument used in _fit_tune_chrom is the order of the multipole to be fitted. It has to be 1 for tunes and 2 for chromaticity, you cannot put anything else, it's the index in PolynomB which will be varied.

Then, this index is used to compute the step in strength to compute the Jacobian. If this step is what you want to control, rather than using the defaults value of delta = 1e-6 * 10 ** index, you need another input.

lfarv commented 10 months ago

An other alternative could be to add the DK to the list in at.lattice.options.DConstants

Right, it could be 2 new constants DK and DH, and corresponding keyword arguments in fit_tune and fit_chrom.

simoneliuzzo commented 10 months ago

Dear @lfarv,

I modified as you suggested.

lfarv commented 10 months ago

By the way, did you have problems with the default step values ?

If you'd like to add the two default values in at.DConstant, it's up to you !

And a final remark: the future release notes will be based on the titles of PRs, so it could be clearer to have something like "Added KStep and HStep arguments to fit_tune and fit_chrom".

simoneliuzzo commented 10 months ago

Dear @lfarv,

thank you for your input! I tried to put everything in.

I prefer not to touch on DConstants. I have nothing against adding those parameters there. I did try to change the DConstants and that is how I found that the tune KStep was not available. So I opened this pull request.

Since fit_chrom was missing **kwargs I added it as in fit_tune.

thank you best regards Simone

lfarv commented 10 months ago

Hi @simoneliuzzo. Looks good to me.

You did not tell why you had to use non-default values for these steps. Playing with niter was not enough to get convergence?

simoneliuzzo commented 10 months ago

Dear @lfarv,

for the FCC-ee lattice, changing quadrupoles by 1e-6 makes the optics unstable. At the moment the tolerances for FCC are "unrealistic" to be kind.

best regards Simone

swhite2401 commented 10 months ago

Can we delete the branch?