Don't get domain dimensions from cparam file for Boxlib frontend. #3470
Conversation
There was a problem hiding this comment.
LGTM, but @zingale might want to weigh-in?
My only concern is the dimensionality here, which we may need to pad for lower-than-3 dimensions.
|
The attached file is 2D and seems to get padded correctly, it should be handled here: # Now we read the global index space, to get
index_space = header_file.readline()
# This will be of the form:
# ((0,0,0) (255,255,255) (0,0,0)) ((0,0,0) (511,511,511) (0,0,0))
# So note that if we split it all up based on spaces, we should be
# fine, as long as we take the first two entries, which correspond to
# the root level. I'm not 100% pleased with this solution.
root_space = index_space.replace("(", "").replace(")", "").split()[:2]
start = np.array(root_space[0].split(","), dtype="int64")
stop = np.array(root_space[1].split(","), dtype="int64")
dd = np.ones(3, dtype="int64")
dd[: self.dimensionality] = stop - start + 1
self.domain_dimensions = dd |
|
LGTM, but I note that the yt/yt/frontends/boxlib/definitions.py Line 39 in e76b39b although it looks like this definitions.py module has fell out of use ? I feel like it's be a good time to maybe clean it up as well. |
|
It looks like nothing from |
|
yup, that's what I'm suggesting. It looks the content was added in 2013 and unless I'm mistaken, |
|
hi folks, I just got back into town from vacation, so I am catching up. If needed, I can test something tonight. |
|
@zingale I think we're mostly letting this open so you can change our minds if this isn't acceptable to you. Maybe you could attempt to load a data file with reduced dimensions and see if all goes well ? Unless you think this could be breaking up any real world case I believe it's okay to not test it too thoroughly. |
|
Do you think this could go in, @zingale? |
|
trying to test it, but seems I hit a yt bug with matplotlib, so I need to figure out how to hack around that |
|
okay, it works for me with a Castro dataset. |
| elif self.dimensionality == 2: | ||
| vals = self.domain_dimensions = np.array([vals[0], vals[1], 1]) | ||
| elif param == "amr.ref_ratio": | ||
| if param == "amr.ref_ratio": | ||
| vals = self.refine_by = int(vals[0]) | ||
| elif param == "Prob.lo_bc": |
There was a problem hiding this comment.
does this work when we specify several different ref_ratios for different jumps between levels? e.g.
amr.ref_ratio = 2 4 2 2
There was a problem hiding this comment.
It "works" in the sense that it will not crash if you give it vector - it takes the first value for the ref_ratio between levels 0 and 1. But I don't think yt itself supports different ref_ratios between different levels.
There was a problem hiding this comment.
I think (hope) it does, since we use it with that all the time.
There was a problem hiding this comment.
That's right, it's not supported yet.
Maybe you'd want to have a warning here in this case, because it's not necessarily obvious from the user standpoint
There was a problem hiding this comment.
In any case, this PR doesn't change anything about the way ref_ratio is handled, so perhaps this could be sorted out in a different issue / pull request?
There was a problem hiding this comment.
agree. That just caught my eye.
|
@meeseeksdev backport to yt-4.0.x |
…file for Boxlib frontend.
…0-on-yt-4.0.x Backport PR #3470 on branch yt-4.0.x (Don't get domain dimensions from cparam file for Boxlib frontend.)
The domain dimensions are always stored in the pltfile Header for Boxlib / AMReX datasets, so reading it from the job_info file is at best redundant, and worst incorrect, in the event that users have chosen to only output data on a subset of the domain.
For example, the attached WarpX plotfile should have domain dimensions of 81, but it gets read as 128 in the following code snippet:
diag200001.tar.gz