ENH: Chimera Frontend

[Ronan-Hix]

PR Summary

This pull request includes a new frontend for use with the CHIMERA code's data format. This code is a grid-based, non-AMR MHD code with spherical geometry and a logarithmic radial coordinate and is utilized for simulations of Type-II core-collapse supernovae in 2 and 3 dimensions. The data required for the answer tests included in the frontend was previously submitted via curl. Development was performed from the basis of the _skeleton frontend, along with advice from John ZuHone and Matt Turk. If this request should be submitted to a different branch, or if there are any other issues, please let me know.

PR Checklist

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

[welcome]

Hi! Welcome, and thanks for opening this pull request. We have some guidelines for new pull requests, and soon you'll hear back about the results of our tests and continuous integration checks. Thank you for your contribution!

[matthewturk]

@Ronan-Hix Thank you! We'll review this ASAP.

[neutrinoceros]

Hi @Ronan-Hix, congratulations on opening this PR, this is an impressive amount of work, especially for a first contribution.

I want to make a couple quick observations:

logarithmic radial coordinate

warning: This isn't supported at the moment in yt (but it's something I really really want to enable too) !

I see your work is already almost passing listing (pre-commit.ci) (impressive!), but there are a couple issues with file permissions you need to fix.

[neutrinoceros]

a very quick and shallow review
note that you've mistakenly included yt/frontends/chimera/tests/.DS_Store. It's light enough that you do not need to rewrite your history but it should be removed.

you can edit your .git/info/exclude file to ignore *.DS_Store files in the future without having to touch the shared .gitignore file

[neutrinoceros]

I guess the new failures are showing (predictable) consequences of the recent switch to unyt for physical constants. We should bump these answers.

[Ronan-Hix]

I think that the failures are actually a product of the slightly changed method of pulling the maximum r-axis limits, but regardless, simply updating the test should still resolve the issue. I'm intending to push another commit soon that will fix this and remove the YinYang implementation from this pull request, as it still needs further development before it is working properly (There are some issues with the rotations in the spherical basis and rasterization problems when doing the rotations in cartesian coordinates).

I was waiting to do so while looking into an issue a collaborator had while using the newest iteration of this frontend in the last couple days; I want to see if this is a package version incompatibility or a true frontend bug I should patch before pushing.

[matthewturk]

@Ronan-Hix Hey -- I just want to say that I'm really grateful for you continuing to plug away on this. Thank you.

[matthewturk]

@yt-fido test this please

[Ronan-Hix]

@Ronan-Hix Hey -- I just want to say that I'm really grateful for you continuing to plug away on this. Thank you.

Absolutely! Sorry for the somewhat intermittent progress, I'm working on this in between a couple other primary research projects and working on grad school applications.

Right now the failing tests come from changing the code behavior to omit ghost zones more reliably (which changes the array size and slightly changes the average). These changes are intentional, so the stored answers should be regenerated, but I'm not quite sure how to do that.

[chrishavlin]

These changes are intentional, so the stored answers should be regenerated, but I'm not quite sure how to do that.

In the tests/tests.yaml you can change local_chimera_001: #PR 3638 to local_chimera_002: #PR 3638 and when you push up, it'll store the current state of the tests as the new answer.

[Ronan-Hix]

I wanted to ping this to see if someone with write access can resolve this conflict. As far as I can see, the versions don’t actually conflict, so I’m not sure how to resolve it on my end.

[neutrinoceros]

done

[neutrinoceros]

We probably need @Xarthisius to sign off on the change he requested

[neutrinoceros]

I'll defer to @chrishavlin and @Xarthisius to finalise their reviews and possibly merge, but AFAICT their respective blocking remarks seem to have been addressed.

[chrishavlin]

Yup, my suggestions are all taken care of. But let's not merge until @Xarthisius looks it over.

One minor thing is that there are a couple of empty files that could be deleted: misc.py, definitions.py.

[neutrinoceros]

One minor thing is that there are a couple of empty files that could be deleted: misc.py, definitions.py.

Right, but that's true for a lot of existing frontends. These modules are actually not required so we could at some point decide to clean empty ones.

[neutrinoceros]

I'm going to close/reopen to refresh CI as requirements for merging have been updated

[welcome]

Hooray! Congratulations on your first merged pull request! We hope we keep seeing you around! 🎆

[neutrinoceros]

Congratulations @Ronan-Hix, and thanks again for your patience. We will include this frontend in the upcoming yt 4.1 release !

[Ronan-Hix]

Very glad to hear it, many thanks to everyone involved for all the assistance!