Skip to content

MPI-AMRVAC support for uniformly stretched grids - #5474

Open
jordidj wants to merge 38 commits into
yt-project:mainfrom
jordidj:amrvac/stretched
Open

MPI-AMRVAC support for uniformly stretched grids#5474
jordidj wants to merge 38 commits into
yt-project:mainfrom
jordidj:amrvac/stretched

Conversation

@jordidj

@jordidj jordidj commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Redefines the AMRVACGrid as a subclass of StretchedGrid and calculates the cell widths' deviation from uniform.

PR Checklist

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

@jordidj
jordidj requested a review from neutrinoceros as a code owner June 28, 2026 18:49
@neutrinoceros

Copy link
Copy Markdown
Member

Hey Jordi, thanks for opening this. My week is a bit bumpy but I'm hoping I can slip in a review in the coming days. Don't hesitate to remind me if I don't make it by next Monday.
In the mean time, could you explain how this relates (or doesn't) to #5212 ?

@neutrinoceros neutrinoceros added enhancement Making something better frontend: amrvac labels Jun 29, 2026
@neutrinoceros neutrinoceros added this to the 4.5.0 milestone Jun 29, 2026
@jordidj

jordidj commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Tbh I forgot it was an open pull request already (#5212), so I redid this from scratch starting from a more recent main because I had issues with continuing from the state of #5212.

@neutrinoceros

Copy link
Copy Markdown
Member

ok so I take it we can close #5212 now ?

@jordidj

jordidj commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Yes, we don't need #5212 anymore

@jordidj

jordidj commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Reading stretch_dim from parfile currently doesn't work if not the entire array is defined, i.e. stretch_dim='uni','','' works, stretch_dim(1)='uni' doesn't.

@neutrinoceros neutrinoceros left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems on the right track. Here are a couple, mostly stylistic suggestions to make it easier to statically check

Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
@jordidj
jordidj requested a review from neutrinoceros July 17, 2026 09:33
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py Outdated
Comment thread yt/frontends/amrvac/data_structures.py
@jordidj
jordidj requested a review from neutrinoceros August 26, 2026 14:56
* Only uniform stretching is supported at the moment, to be defined
as ``'uni'`` or ``'uniform'``.
* At present, stretched grids are only supported on a
single level of refinement.

@jordidj jordidj Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This last bullet point was moved here from below, but I don't know if it is still true. At least this test case (7 levels) looks fine at a glance.

Image

@jordidj

jordidj commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Should I add a test? If so, can you point me in the right direction on how to set that up?

@neutrinoceros

Copy link
Copy Markdown
Member

A test would indeed help. The process is a bit involved though, and I'm not even sure we can still do it as the server we used to run integration tests against real data died a couple months back... I'll ask around.

@neutrinoceros

Copy link
Copy Markdown
Member

The answer I got is that the dataset sample collection is still up and can be extended, so we should be able to add a test !
I don't know that the whole process is documented, but the gist of it is that you'll need to upload a data sample as a tarball using yt upload from the command line, then open a PR at yt-project/website to update the registry in the style of: yt-project/website#129
Finally, you'll need to update yt/sample_data_registry.json correspondingly, either here or as a separate PR, and obviously write a test to check that the grid is parsed as expected.
Maybe you'll want to add an image-comparison test instead, or in addition to it (not required). That's another layer of complexity but thankfully I documented it, so hopefully that's useful https://yt-project.org/docs/dev/developing/testing.html#creating-and-updating-image-baselines-for-pytest-mpl-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Making something better frontend: amrvac

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants