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
fix bug in frequency map when tune above 0.5 #745
base: master
Are you sure you want to change the base?
Conversation
Dear @swhite2401, |
Hi @oscarxblanco , I can review it if you are a bit patient, the coming week is quite busy for me. Is it ok for you? |
@swhite2401 , no problem at all. This is a minor bug. From experience this should be solved with a PEP8 check. Is there any PEP8 application that you suggest ? In another disscussion I read @lfarv uses black (https://black.readthedocs.io/en/stable/), right ? |
@oscarxblanco: Concerning PEP8 conformance, it's a matter of coding style, but it has no effect on the run time behaviour of the code. The test sequence does include a PEP8 conformance test (5th step called "Lint with flake8"), but it does not affect the success of the sequence. It just prints out a list of all non-conforming lines, which I think very few people look at. Now to ensure PEP8 conformance, I'm using PyCharm for development, which permanently displays warnings about all non-conforming code. But it's then put to you to correct. And you are right, for some time now, I've been trying "black" on new or modified files. I'm quite happy with it, with the following advantages:
I was about to post a pull request modifying the "developer notes" of the documentation to add a recommendation on how to install and run "black" for code formatting. What do you think of that? |
Dear @lfarv , Installation is easy through pip, and running it seems simple. I tested it in a 100 lines python script and it corrects the length of the lines automatically... it also seems possible to activate or desactivate in certain regions through pragma commands. |
@oscarxblanco , the implementation seems fine to me. However, should we keep this feature as optional? |
Dear @swhite2401 , There will be a warning if any of the ring transverse tunes is above 0.5 and ring.get_tune()=array([0.81707699, 0.73766671])
Warning: at least one tune is above 0.5, and nu0p5 flag is False The frequency map data is modified only if Here again is a test code, it is the same as before but adding the '''
This file is used to test the ring tune above 0p5
oblanco 2024mar01
'''
import at
import numpy
print(f'{at.__version__}=')
QF=at.Quadrupole('QF',0.5,1.2)
Dr = at.Drift('Dr', 0.5)
HalfDr = at.Drift('Dr2', 0.25)
QD = at.Quadrupole('QD', 0.5, -1.2)
Bend = at.Dipole('Bend', 1, 2*numpy.pi/30)
ring = at.Lattice(3*[HalfDr, Bend, Dr, QF, Dr, Bend, Dr, QD, HalfDr],
name='Simple FODO cell', energy=1E9)
print(f'{ring.get_tune()=}')
eoffset=0
pool_size=12
turns=1024
fmadata = at.fmap_parallel_track(ring,
coords=[-10,10,-10,10],
add_offset6D=numpy.array([0,0,0,0,eoffset,0]),
steps=[3,3],
pool_size=pool_size,
turns=turns,
verbose=False,
nu0p5=True);
fmadata=numpy.array(fmadata[0])
nuxlist = fmadata[:,2]
nuylist = fmadata[:,3]
print(f'{nuxlist=}')
print(f'{nuylist=}') @swhite and @lfarv , do you consider mandatory to execute black on this script ? Best regards, |
Dear all, this PR fixes a bug.
The fractional tune returned by the frequency analysis lies between 0 and 0.5, which, mismatches the tune representation in AT (0 to 1), and would be visible in lattices with any tune above 0.5.
In this PR, the fractional tune returned by the frequency analysis is corrected as 1 minus the value when the lattice tune is above 0.5.
This should solve the issues in most cases except when the tune footprint crosses 0, 0.5 or 1.
I leave here the code to do a quick test: