ocelot-collab / ocelot

OCELOT is a multiphysics simulation toolkit designed for studying FEL and storage ring-based light sources.
GNU General Public License v3.0
84 stars 58 forks source link

Question: is the use of cpbd.io._find_objected deprecated? #169

Closed smartsammler closed 1 year ago

smartsammler commented 1 year ago

Hello,

in MR https://github.com/ocelot-collab/ocelot/pull/72 (@michael-boese and @sergey-tomin) more specific in commit https://github.com/ocelot-collab/ocelot/commit/50b8d228c39c73511e6ba5abf2ece523d45c26c4 the public function ocelot.cpbd.io.find_objects was renamed to _find_objects indicating it is private. Does this imply the use of it is discouraged? Can one use latticeIO.LatticeIO._find_objects instead of find_objects or how should one replace the usage of it now? Unfortunately, I couldn't find any information about this change in the Changelog.

We use it to iterate over elements and adjust certain properties, e.g.

for el in find_objects(lattice, [SBend]):
    el.angle = np.deg2rad(360 / 16)

Thanks in advance.

st-walker commented 1 year ago

Michael left about a year ago, so why this was done we may never know (unless @sergey-tomin has an idea). The changelog is a nice idea but is sadly only rarely updated. I think it was born after Michael did his Great Refactoring.

LatticeIO is a not so maintained area of the codebase. That it consists of a single class with only static methods and no state is testament to this. Its purpose is for writing Python files from OCELOT lattices, as I understand. I suppose Michael wasn't expecting people to use a module for writing to edit magnet properties.

What I would suggest instead:

sbends = [x for x in lattice.sequence if isinstance(x, SBend)]
for sbend in sbends:
   sbend.angle = some_angle_i_want_it_to_be

or using an instance method on MagneticLattice:

mlat = MagneticLattice(...)
mlat.find_indices(SBend)
# or
mlat.find_indices_by_predice(some_callable)

then loop over the indices and use them to access the sbends

sergey-tomin commented 1 year ago

Hi @smartsammler,

When I wrote the find_object() function, I didn't envision it being used in this way, so we just put it in the LatticeIO class where it belongs. @st-walker pointed out the way we most often use to find elements by type. Perhaps we need this function in MagneticLattice...

smartsammler commented 1 year ago

Thank you very much for the answers including the suggestions and background!