slaclab / lume-genesis

Genesis 1.3 tools for use in LUME
https://slaclab.github.io/lume-genesis/
Apache License 2.0
4 stars 7 forks source link

REF/ENH: rework Genesis 4 support completely (lark grammar-based parser + pydantic) #17

Closed ken-lauer closed 4 weeks ago

ken-lauer commented 6 months ago

Description

TODO / plan details

Chris TODO

Future (not in the scope of this PR)

ChristopherMayes commented 6 months ago

This is for Genesis4 only, so the lark and grammar files should go in genesis/version4

ChristopherMayes commented 6 months ago

How were the data classes made? I would be better if the comments were part of the docstring.

ken-lauer commented 6 months ago

How were the data classes made?

I didn't get to the JSON version of the upstream documentation as I was waiting on the other PR. I think hand-crafted will always win when compared to auto-generated - especially in the case where these things don't change very often - but that's another thing we can discuss and weigh up.

I would be better if the comments were part of the docstring.

For sphinx, #: syntax will include such comments as part of the generated documentation for each attribute.

It's partly a stylistic choice, partly a functional one. Personally, I think keeping the description close to the declaration rather than away in a docstring that you have to search through (and may more easily get out of sync) is preferable.

ChristopherMayes commented 6 months ago

I'd like to have the field descriptions accessible without having to go to the source file, so they should at least be in the docstring. Similar to the main input structures, I think we can automate all of this from JSON info, I'm not sure how much needs to be handcrafted.

ken-lauer commented 6 months ago

Turns out there are a bunch of parameters missing in the documentation for the main input file format (specifically "track"; others will have to be audited similarly). I'll start to fix the upstream documentation first to get this fully compliant - though I'm not sure I'll be able to figure out the parameter descriptions on my own.

Edit: Here's a few more, found by grepping and checking versus our dataclasses in this PR:

Altersetup parameter disable missing
Setup parameter outputdir missing
Setup parameter exclude_field_dump missing
Setup parameter write_meta_file missing
Setup parameter write_semaphore_file missing
Setup parameter write_semaphore_file_done missing
Setup parameter write_semaphore_file_started missing
Track parameter s0 missing
Track parameter slen missing
Track parameter field_dump_at_undexit missing
Track parameter bunchharm missing
Track parameter dbg_report_lattice missing
Track parameter dbg_suppress_outfile missing

Edit 2:

Second audit of sorts - missing parameters listed in namelist "usage()" functions:

Missing field for class Profile_file: autoassign
Missing field for class Sequence_power: seed
Missing field for class Sequence_power: normal
Missing field for class Importdistribution: settimewindow
Missing field for class Importdistribution: eval_start
Missing field for class Importdistribution: eval_end
Missing field for class Importdistribution: align
Missing field for class Importdistribution: align_start
Missing field for class Importdistribution: align_end
Missing field for class Setup: outputdir
Missing field for class Setup: exclude_field_dump
Missing field for class Setup: write_meta_file
Missing field for class Setup: write_semaphore_file
Missing field for class Setup: write_semaphore_file_done
Missing field for class Setup: write_semaphore_file_started
Missing field for class Setup: semaphore_file_name
Missing field for class Altersetup: string
Missing field for class Altersetup: disable
Missing field for class Track: s0
Missing field for class Track: slen
Missing field for class Track: field_dump_at_undexit
Missing field for class Track: bunchharm
Missing field for class Efield: dump

Also: alter_setup is named incorrectly (patching manual here; will propose upstream as well)

Need to double-check those against the parsers...

ChristopherMayes commented 6 months ago

That's okay. Put in a placeholder to be filled out later.

ChristopherMayes commented 1 month ago

I see a Numpy v2 error running the Example1 notebook:

AttributeError: `np.string_` was removed in the NumPy 2.0 release. Use `np.bytes_` instead.
ken-lauer commented 1 month ago

I see a Numpy v2 error running the Example1 notebook:

AttributeError: `np.string_` was removed in the NumPy 2.0 release. Use `np.bytes_` instead.

There isn't a string_ in this codebase; it may be coming from another library (did you upgrade openpmd-beamphysics?).

ChristopherMayes commented 1 month ago

@ken-lauer sorry, I was using the wrong branch.

ChristopherMayes commented 1 month ago

This has 96 hardcoded, which is too high for most people. Also it's inconsistent with text above:

image
ken-lauer commented 1 month ago

This has 96 hardcoded, which is too high for most people

There's a warning above:

Warning this example requires a considerable amount of memory (over 100GB) and processor cores in order to be run in a reasonable amount of time. Your laptop may not be sufficient.

ChristopherMayes commented 1 month ago

What about the text about 12 cores in there?

ken-lauer commented 1 month ago

What about the text about 12 cores in there?

Where?

ChristopherMayes commented 1 month ago
image