Skip to content

Commit

Permalink
Add optional argument to make sure xbout reads David Bold's MMS outpu…
Browse files Browse the repository at this point in the history
…t file as a dump file rather than a grid file.
  • Loading branch information
mrhardman committed Dec 6, 2024
1 parent c7dd274 commit 0ff4e76
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions xbout/load.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ def open_boutdataset(
run_name=None,
info=True,
is_restart=None,
is_mms_dump=False,
**kwargs,
):
"""
Expand Down Expand Up @@ -180,12 +181,12 @@ def open_boutdataset(
chunks = {}

input_type = _check_dataset_type(datapath)

if is_restart is None:
is_restart = input_type == "restart"
elif is_restart is True:
input_type = "restart"

if is_mms_dump:
input_type = "dump"
if "reload" in input_type:
if input_type == "reload":
if isinstance(datapath, Path):
Expand Down Expand Up @@ -633,7 +634,6 @@ def _auto_open_mfboutdataset(
nxpe, nype, mxg, myg, mxsub, mysub, is_squashed_doublenull = _read_splitting(
filepaths[0], info, keep_yboundaries
)

if is_squashed_doublenull:
# Need to remove y-boundaries after loading: (i) in case we are loading a
# squashed data-set, in which case we cannot easily remove the upper
Expand Down Expand Up @@ -737,9 +737,10 @@ def _auto_open_mfboutdataset(
)

if not is_restart:
# Remove any duplicate time values from concatenation
_, unique_indices = unique(ds["t_array"], return_index=True)
ds = ds.isel(t=unique_indices)
if "t_array" in ds.keys():
# Remove any duplicate time values from concatenation
_, unique_indices = unique(ds["t_array"], return_index=True)
ds = ds.isel(t=unique_indices)

return ds, remove_yboundaries

Expand Down

5 comments on commit 0ff4e76

@mikekryjak
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mrhardman the reason that xBOUT is thinking that you don't have a dump file is because it doesn't have a time output.
This is how xBOUT decides the file type:

xBOUT/xbout/load.py

Lines 593 to 595 in d113e0e

elif "t" in ds.dims:
# (iii)
return "dump"

t is missing in the MMS test output. I would recommend to undo the changes from this commit and add time as an output variable to the MMS test.

@mrhardman
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We discussed this on slack, and @dschwoerer was not in favour of this solution. Did you test that manually adding a t dimension solved all subsequent issues? If not, we will need to check if the dataset can have a dimension which none of the arrays use.

@mikekryjak
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I just reviewed the chat. In that case why don't we rename this flag to "is_dump" or something like that? It sounds like it could be useful for whoever writes a BOUT++ application that doesn't need a time array.

@dschwoerer
Copy link
Contributor

Choose a reason for hiding this comment

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

Might it not be better to always add some metadata to the BOUT++ output files?
Having xBOUT guess the type is probably not that great in general, that is why I was against this option, as it might be for other tools also helpful to have the info what kind of data this is.

Then we do not need an option for xBOUT, as the dump files will always have the needed info.

@mikekryjak
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dschwoerer I totally agree that this would be the right solution. But in the meantime, maybe we can still put this flag in?

Please sign in to comment.