Add `Parthenon` frontend

[pgrete]

PR Summary

Add yt frontend for codes based on the Parthenon AMR framework (https://github.com/parthenon-hpc-lab/parthenon) including AthenaPK (https://github.com/parthenon-hpc-lab/athenapk), Phoebus (https://github.com/lanl/phoebus) or KHARMA (https://github.com/AFD-Illinois/kharma) Note that the mesh structure in Parthenon is heavily derived from Athena++, so that's why we used the Athena++ frontend as baseline.

We've been using this frontend for quite some time now (without any recent major changes) so we consider it stable enough to open it for review and merge to yt proper.

At this point I'm looking for

• general feedback
• feedback on how to generally handle/differentiate between Parthenon (as framework) and downstream code related functions/fields/... (the current version of this frontend is geared towards AthenaPK and I'd like to abstract this)
• (assuming a test dataset is desired), how to upload/handle/provide the dataset

PR Checklist

• [ ] New features are documented, with docstrings and narrative docs
• [ ] Adds a test for any bugs fixed. Adds tests for new features.

[matthewturk]

This is awesome! Thank you for contributing it. I'll provide a review ASAP.

[neutrinoceros]

Congratz @pgrete ! I'll make a first pass for QA and will try to focus on stuff that isn't already reported by linters (see pre-commit.ci's report).

[neutrinoceros]

• feedback on how to generally handle/differentiate between Parthenon (as framework) and downstream code related functions/fields/... (the current version of this frontend is geared towards AthenaPK and I'd like to abstract this)

I think we have one example of a framework-as-a-dataformat already: the boxlib frontend. In my experience, t's indeed the one for which automatic determination of exact format is hardest to implement and maintain, and the reason why we added a hint keyword argument to yt.load. I hope we'll be able to have a cleaner distinction between formats with this frontend but if the binary data format is to be identical I think one practical approach is to reserve some header in binary files to encode some unique key (like the name of the code) that maps to a known format. Otherwise it may be impossible to guess.

At this point, and I'm not saying that to discourage this pull request, you may want to consider distributing the frontend as an extension package, which can help if frequent contributions are expected and you want to be free from our relatively slow release cycle.

• (assuming a test dataset is desired), how to upload/handle/provide the dataset

yes that would be a necessary step for us to merge this PR. The process requires a companion PR to our website repo. The most recent example of such a PR would be https://github.com/yt-project/website/pull/115

[pgrete]

• (assuming a test dataset is desired), how to upload/handle/provide the dataset

yes that would be a necessary step for us to merge this PR. The process requires a companion PR to our website repo. The most recent example of such a PR would be yt-project/website#115

Regarding the dataset, am I assuming correctly that the hdf5 file should not use (default) compression, i.e., deflate?

[matthewturk]

@pgrete I think as long as it doesn't require hdf5plugin it should be OK to use deflate.

[neutrinoceros]

I think that the reason CI is now failing is that #4344 was merged in between runs. Add test_outputs.py to the list of files that pytest should ignore to resolve this.

[pgrete]

I think that the reason CI is now failing is that #4344 was merged in between runs. Add test_outputs.py to the list of files that pytest should ignore to resolve this.

I'm currently figuring out some useful tests to add (with the appropriate data file).

[pgrete]

Looks like I'm stuck. How can I actually run tests?

I first thought that the test are answer tests so that I should call them using a class (like TestTipsy described here https://yt-project.org/doc/developing/testing.html#how-to-run-the-answer-tests) but then I realized that the current test_outputs.py does not define a class (or I don't see where it does) -- similar to other test_outputs.py of other frontends.

Then I tried pytest (after reinstalling the project to fix dependencies) and only got

$ pytest yt/frontends/parthenon/tests
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.10.9, pytest-7.2.2, pluggy-1.0.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/pgrete/src/yt, configfile: pyproject.toml
plugins: typeguard-2.13.3, anyio-3.6.2
collected 0 items                                                                                                                                                                            

=================================================================================== no tests ran in 0.01s ====================================================================================

so what am I missing?

[pgrete]

I'm a little stuck here. I realized that the failing test was run with py38 so I thought that my python environment may be too new (also wrt the nose PR for python >3.10).

So I installed a clean env with python 3.8 and the "full" version of yt pip install -e .[full]

Trying the test now results in:

$ pytest yt/frontends/parthenon/tests/test_outputs.py 
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.8.16, pytest-7.2.2, pluggy-1.0.0 -- /home/pgrete/miniconda3/envs/py3.8/bin/python
cachedir: .pytest_cache
rootdir: /home/pgrete/src/yt, configfile: pyproject.toml
collected 0 items / 1 error                                                                                                                                                                  

=========================================================================================== ERRORS ===========================================================================================
_______________________________________________________________ ERROR collecting yt/frontends/parthenon/tests/test_outputs.py ________________________________________________________________
yt/frontends/parthenon/tests/test_outputs.py:11: in <module>
    from yt.utilities.answer_testing.framework import (
yt/utilities/answer_testing/framework.py:24: in <module>
    from nose.plugins import Plugin
../../miniconda3/envs/py3.8/lib/python3.8/site-packages/nose/__init__.py:1: in <module>
    from nose.core import collector, main, run, run_exit, runmodule
../../miniconda3/envs/py3.8/lib/python3.8/site-packages/nose/core.py:12: in <module>
    from nose.loader import defaultTestLoader
../../miniconda3/envs/py3.8/lib/python3.8/site-packages/nose/loader.py:21: in <module>
    from nose.importer import Importer, add_path, remove_path
../../miniconda3/envs/py3.8/lib/python3.8/site-packages/nose/importer.py:12: in <module>
    from imp import find_module, load_module, acquire_lock, release_lock
../../miniconda3/envs/py3.8/lib/python3.8/imp.py:31: in <module>
    warnings.warn("the imp module is deprecated in favour of importlib; "
E   DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
================================================================================== short test summary info ===================================================================================
ERROR yt/frontends/parthenon/tests/test_outputs.py - DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
====================================================================================== 1 error in 0.29s ======================================================================================

What am I missing?

[neutrinoceros]

Sorry for the late reply. GenericArrayTest is a subclass of AnswerTestingTest and as such, it cannot be run with pytest yet (we are still in the middle of the migration process). As a result, your tests can only be run with Python 3.8 or 3.9 (since nose isn't compatible with Python 3.10 and later).

[pgrete]

So I'd call python tests/nose_runner.py (as that's the command being run by the py38 GitHub check)? And, if yes, can I just run the Parthenon frontend or do I need to run all tests?

[neutrinoceros]

test/nose_runner.py is a connivence script. I'm sure it's possible to run only a fraction of tests. I'm actually very confused seeing the docs recommend using pytest for that https://yt-project.org/docs/dev/developing/testing.html#how-to-run-the-answer-tests

so clearly I'm not the expert here. Does the documentation help ? if not, maybe @Xarthisius is would be of better guidance.

[pgrete]

I figured it out (for reference nosetests -v --with-answer-testing --local --local-dir $HOME/src/yt-test-data --answer-store --answer-name=local-parthenon yt.frontends.parthenon). Going back to the old doc helped.

I just added a plain Parthenon 2D test (which of course will fail given that the data is not available online).

What do the next steps look like in general (in terms of when to upload/open a PR for the reference files) and the combined review?

With respect to this PR, I plan to make the following two additions:

• a 3D example (of the 2D type above) but with dx != dy != dz
• a 3D AthenaPK example (incl changes to work with a code hint) with support for units

Do you recommend any other additional tests? Is sth else required to complete this PR?

[neutrinoceros]

What do the next steps look like in general (in terms of when to upload/open a PR for the reference files) and the combined review?

AFAICT it seems mature enough that submitting datasets for upload now wouldn't be inappropriate.

Do you recommend any other additional tests?

I think that frontend tests should be as focused as possible on things that might go wrong specifically with the data format at hand. Tests exposing problems encountered during development are also very welcome, but don't feel like you need to be 100% exhaustive.

Is sth else required to complete this PR?

Eventually we'll want to have 2 approvals from maintainers, but you're close to getting mine already, since my initial comments were all addressed ! I'll need to review again, and I'm ready to do so as soon as needed. Feel free to ping me if you'd rather get my review before dataset are uploaded, otherwise I'll just wait for tests to go green !

[pgrete]

In the process of adding data files of two other codes I'm currently wondering a) if (and how) yt might handle data from general relativistic simulations, i.e., data that might carry a metric around b) if (and how) multi material simulation data is supported (e.g., different equations of state for different fields)

[matthewturk]

@pgrete For a), this should be feasible (@chrishavlin has done some on-the-same-line-of-thought work with integrating pressure to get spatial coordinates) but might need some collaborative thinking, which I'm up for. For b) this should be feasible by even in a worst case using multiple "field types". Either way, both are high priorities, so let's try!

[forrestglines]

Just pushed some quick changes so that coordinate ordering for cylindrical and spherical coordinates follows Athena++'s conventions, which AthenaPK will inherit.

(This is the last functional addition we had planned for this PR)

[chrishavlin]

just FYI, i hit the button for the tests to run, but you'll likely get unrelated failures after your merge with main (e.g., #4785 ).

[forrestglines]

I've pushed @neutrinoceros 's bug fix to this PR although I'm still working on Polar. You can advise on how you'd like me to fix the git history later on if the merges get messy.

[forrestglines]

One aspect of Geometry.SPHERICAL I'd like to verify -- the ordering is $(r,\theta,\phi)$, right? Where $\theta$ is the polar angle $[0,\pi]$ and $\phi$ is the azimuthal angle $[0,2 \pi]$?

This would be consistent with Parthenon's ordering.

[neutrinoceros]

I've pushed @neutrinoceros 's bug fix to this PR although I'm still working on Polar. You can advise on how you'd like me to fix the git history later on if the merges get messy.

I think we'll need to rebase this branch at least partially to mitigate merge confusion in the future. I would advise not to merge from main until then, or it might become very hard to just manage this branch.

One aspect of Geometry.SPHERICAL I'd like to verify -- the ordering is $(r,\theta,\phi)$, right? Where $\theta$ is the polar angle $[0,\pi]$ and $\phi$ is the azimuthal angle $[0,2 \pi]$?

Yes. You can check that by grepping for _default_axis_order and see that SphericalCoordinateHandler has _default_axis_order = ("r", "theta", "phi"). Also note that these represent default orders, but it's possible to specify different ones where needed, and still use the builtin CoordinateHandler classes ! (though admittedly "theta" and "phi" are pretty ambiguous notations, but I can confirm that within yt, $\theta$ is colatitude and $\varphi$ is azimuth)

[pgrete]

I'm back a little bit late to the show so I'm wondering why there may be merge conflicts? I regularly updated this branch with main and things have been working fine so far.

[matthewturk]

Hm, I don't see any merge conflicts

[forrestglines]

I'm back a little bit late to the show so I'm wondering why there may be merge conflicts? I regularly updated this branch with main and things have been working fine so far.

I merged in #4790 to try to fix Geometry.POLAR but that PR is still not working. I'm considering undoing that merge and just using Geometry.CYLINDRICAL like the Athena++ frontend does.

Otherwise, the git history is messy. I was planning on nuking the git history if things got too messy since not too many of us are working on this.

[matthewturk]

Otherwise, the git history is messy. I was planning on nuking the git history if things got too messy since not too many of us are working on this.

I'm personally not a huge stickler for neat/tidy git history, but I know it is something others prioritize. I think when this is merged, we can also squash it, or if you'd like you can edit the history into manageable and discrete chunks before that.

[forrestglines]

We have these ~~two~~ three files for code verification so far

advection_2d.out0.final.phdf

http://use.yt/upload/8f07e626

athenapk.prim.disk.phdf (Edited to cover a few more tests)

http://use.yt/upload/fd82672e

athenapk.prim.cluster.phdf

http://use.yt/upload/d8d549e5

[forrestglines]

I'm personally not a huge stickler for neat/tidy git history, but I know it is something others prioritize. I think when this is merged, we can also squash it, or if you'd like you can edit the history into manageable and discrete chunks before that.

I'll clean up the git history or at least remove the commits from #4790 that are not used.

With respect to this PR, I plan to make the following two additions:

* a 3D example (of the 2D type above) but with dx != dy != dz

* a 3D AthenaPK example (incl changes to work with a code hint) with support for units

@pgrete do we want to add these tests? The tests I just added I think would address everything that is specific to downstream code datasets:

1. Loading in an AthenaPK dataset with the Parthenon frontend
2. Loading units from the phdf file
3. Creating derived fields (like temperature) if the right fields are in the phdf file
4. Loading in a cylindrical dataset

I could see added more testing for magnetic fields and datasets with dx != dy != dz. Beyond that, I think this is ready for review again

[forrestglines]

I added more tests, this PR is finished except for review and requested changes.

[BenWibking]

I am getting an error when using the latest version of this branch to load an AthenaPK dataset:

File ~/yt_parthenon/yt/loaders.py:99, in load(fn, hint, *args, **kwargs)
     58 """
     59 Load a Dataset or DatasetSeries object.
     60 The data format is automatically discovered, and the exact return type is the
   (...)
     95     If the data format matches more than one class of similar specialization levels.
     96 """
     97 from importlib.metadata import entry_points
---> 99 from yt.frontends import _all  # type: ignore [attr-defined] # noqa
    101 _input_fn = fn
    102 fn = os.path.expanduser(fn)

File ~/yt_parthenon/yt/frontends/__init__.py:57, in __getattr__(value)
     54 if value == "_all":
     55     # recursively import all frontends
     56     for _ in __all__:
---> 57         __getattr__(_)
     58     return
     60 if value not in __all__:

File ~/yt_parthenon/yt/frontends/__init__.py:63, in __getattr__(value)
     60 if value not in __all__:
     61     raise AttributeError(f"yt.frontends has no attribute {value!r}")
---> 63 return importlib.import_module(f"yt.frontends.{value}.api")

File ~/.conda/envs/yt-analysis/lib/python3.11/importlib/__init__.py:126, in import_module(name, package)
    124             break
    125         level += 1
--> 126 return _bootstrap._gcd_import(name[level:], package, level)

File ~/yt_parthenon/yt/frontends/ramses/api.py:2
      1 from . import tests
----> 2 from .data_structures import RAMSESDataset
      3 from .definitions import field_aliases
      4 from .fields import RAMSESFieldInfo

File ~/yt_parthenon/yt/frontends/ramses/data_structures.py:35
     33 from .field_handlers import get_field_handlers
     34 from .fields import _X, RAMSESFieldInfo
---> 35 from .hilbert import get_intersecting_cpus
     36 from .io_utils import fill_hydro, read_amr
     37 from .particle_handlers import get_particle_handlers

File ~/yt_parthenon/yt/frontends/ramses/hilbert.py:6
      3 import numpy as np
      5 from yt.data_objects.selection_objects.region import YTRegion
----> 6 from yt.geometry.selection_routines import (
      7     bbox_intersects,
      8     fully_contains,
      9 )
     11 # State diagram to compute the hilbert curve
     12 _STATE_DIAGRAM = np.array(
     13     [
     14         [
   (...)
     46     ]
     47 )

ImportError: cannot import name 'bbox_intersects' from 'yt.geometry.selection_routines' (/u/bwibking/yt_parthenon/yt/geometry/selection_routines.cpython-311-x86_64-linux-gnu.so)

[matthewturk]

@BenWibking That's usually what shows up if we add a new function to the Cython code and the library isn't rebuilt. I'm not sure how you install, but for me it's usually a matter of python setup.py build_ext -i in the yt source directory.