atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

Preserve the case of RingParam attribute names #744

Closed lfarv closed 6 months ago

lfarv commented 7 months ago

This corrects a bug reported in #743: the name all unexpected attributes in a Matlab RingParam element is converted to lower case in the python Lattice object. When saving back to .mat or .m file, the attribute name is restored to a new RingParam element by capitalising its 1st letter. So the restored name may differ from the original name.

Now, the names of all unexpected attributes are kept unchanged between the Matlab RingParam element and the python Lattice object.

oscarxblanco commented 7 months ago

Dear @lfarv , there is an issue when saving mat files. See below the test code I wrote and the error output.

'''
This file is used to test the ring parameters handling
oblanco 2024mar01
https://github.com/atcollab/at/pull/744
'''

import at
import numpy

print(f'{at.__version__}=')

# from pyat primer
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/40)
ring1 = at.Lattice([HalfDr, Bend, Dr, QF, Dr, Bend, Dr, QD, HalfDr],
                      name='Simple FODO cell', energy=1E9)

# test
at.save_mat(ring1,'ring1.mat')
ring2 = at.load_mat('ring1.mat')

Here is the error


>>> exec(open('testringparam.py').read())
0.5.1.dev20+g87e43366=
/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/particle_object.py:35: UserWarning: AT tracking still assumes beta==1
Make sure your particle is ultra-relativistic
  warn(UserWarning("AT tracking still assumes beta==1\n"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 22, in <module>
  File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/load/matfile.py", line 181, in load_mat
    return Lattice(
           ^^^^^^^^
  File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/lattice_object.py", line 219, in __init__
    self.update(kwargs)
  File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/lattice_object.py", line 514, in update
    setattr(self, key, value)
  File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/lattice_object.py", line 716, in particle
    self._particle = Particle(particle)
                     ^^^^^^^^^^^^^^^^^^
  File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/particle_object.py", line 37, in __init__
    if name in self._known:
       ^^^^^^^^^^^^^^^^^^^
TypeError: unhashable type: 'dict'
``
lfarv commented 7 months ago

Hi @oscarxblanco Thanks for testing: things are more complicated than expected. I had to cleanup the processing of Lattice attributes, which also revealed a few small bugs, now corrected. This version should be correct, let me know.

oscarxblanco commented 7 months ago

Dear @lfarv ,

I have finished my tests saving in pyat and AT matlab to .mat and .m files with EApertures and RApertures, and with and w/o the flag keep_all on the pyat functions at.load_mat and at.load_m. All results are OK.

I attach to this message a zip file containing the scripts I used for test.

This is OK to merge on my side.

testringparams_and_apertures.zip

lfarv commented 6 months ago

@oscarxblanco:

I cannot find your scripts in your last comments… But since you can test carefully, I did some more cleaning, could you please check again ? Then I'll need another approval!

Do you think that your test, or part of it, could be added to the automatic test sequence ?