search

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

Add string key aliases for output field harmonics (`fieldN_key` N > 1) #35

Closed ken-lauer closed 1 month ago

ken-lauer commented 2 months ago
  1. This power key catch-all is back-compat that should go away from Chris's original code:

https://github.com/ken-lauer/lume-genesis/blob/a15c20ec1d4c3fe2a4c60496619fe09c8a7cd8c4/genesis/version4/output.py#L1618-L1622

  1. Then we need to add string aliases for field harmonics beyond the first one, which are present in field_harmonics[N].attr
  2. Chris indicated that field3_energy (for example) would be a good pattern here
ken-lauer commented 2 months ago

@ChristopherMayes I've added support for these aliases (in #41) but there are some back-compatibility string key handling lines that need fixing. I'd like your thoughts as to how best to handle these 2 cases:

  1. https://github.com/slaclab/lume-genesis/blob/469fe0918c757de2aadcc20bb7373837354f7f58/genesis/version4/output.py?plain=1#L1701-L1703 (output.stat("power"))
  2. https://github.com/slaclab/lume-genesis/blob/469fe0918c757de2aadcc20bb7373837354f7f58/genesis/version4/output.py?plain=1#L1629-L1633 (plot(y=["power"]))

Currently, anything with "power" in it will collapse into field_harmonics[1].peak_power. This is clearly incorrect. How should this be handled so that there will be the least amount of confusion for users?

(Changing this may come as a surprise to users who incorrectly have an incorrect alias with "power" in it, but I think it's important to fix regardless)

ChristopherMayes commented 2 months ago

There's no way to make sense of averaging the field power for creating the stats data, so I had 'corrected' the request for plotting power as the peak power. Now we should probably just disallow that, and instead have the user request peak_power from the stats. We could throw in a helpful message in the error.

ken-lauer commented 2 months ago

There's no way to make sense of averaging the field power for creating the stats data, so I had 'corrected' the request for plotting power as the peak power. Now we should probably just disallow that, and instead have the user request peak_power from the stats. We could throw in a helpful message in the error.

Attempt at that is here: https://github.com/slaclab/lume-genesis/pull/41/commits/cf1b1c4b3a95ced89ecb7153e28a8208e155005b