Skip to content
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

Add hook for submodules for extensions #249

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

dschwoerer
Copy link
Contributor

Alternative to xHERMES :-)

It allows to use xBOUT for loading any BOUT++ simulation. That way I don't have to think what simulation has generated the data. I just load it, and as long as an appropriate extension is installed, e.g. https://github.com/dschwoerer/xbout.modules.hermes for hermes (as a proof of concept).

After all, I don't want to do something hermes specific. I just want to have decent units ...

If there are some post-processing that would only ever a STORM user do, sure.

But I rather feel the other way round. I would like to have the same interface for my EMC3 simulations and my BOUT++ simulations. But the ones are ds.emc3.* the others ds.bout.* - I want to avoid this to go worse ...

Note that there is a pip bug with submodules and editable installs: pypa/pip#11467

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2022

Codecov Report

Merging #249 (e435cc7) into master (30fa7a7) will decrease coverage by 0.24%.
The diff coverage is 45.45%.

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   74.11%   73.86%   -0.25%     
==========================================
  Files          13       14       +1     
  Lines        2503     2525      +22     
  Branches      594      609      +15     
==========================================
+ Hits         1855     1865      +10     
- Misses        428      435       +7     
- Partials      220      225       +5     
Impacted Files Coverage Δ
xbout/load.py 74.00% <27.27%> (-1.31%) ⬇️
xbout/modules/__init__.py 63.63% <63.63%> (ø)
xbout/plotting/animate.py 46.08% <0.00%> (ø)
xbout/plotting/plotfuncs.py 48.27% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bendudson
Copy link
Contributor

Thanks @dschwoerer ! I do agree that a generic solution here would be better, if it can be done.

One thing that can be done is the unit conversion and data labels. xhermes and xstorm use attributes:
units : string containing the SI units e.g. "kg m^2 / s"
conversion : float. Multiply the raw value by this to get SI units
standard_name : A short name like "density", "velocity" that should be standardised
long_name : A longer description suitable for axis labels and legends

e.g.
https://github.com/boutproject/xhermes/blob/main/xhermes/load.py#L67

Those attributes could be in the BOUT.dmp files, enabling automatic conversion

@dschwoerer
Copy link
Contributor Author

I think that would make most of the pluggins work unnecessary 🎉

Do we want to say "SI" - or are we happy with eV as temperature?

All I would be missing is ds["Te"] = ds.Pe / ds.Ne - not sure this can reasonably be generalised?

The other thing that may be needed is if the log of a quantity is evolved. I think STORM is doing that ...

@ZedThree
Copy link
Member

I would say that as long as the units are recognised by UDUNITS or perhaps pint then it should be ok -- eV for temperature should be accepted for instance.

That might also extend to log units too, but I suspect that might be trickier or just not exist in any existing convention -- so you'd either need to just document it somewhere, define new units, or perhaps even just only write out the non-log form?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants