Skip to content

WriterADIOS2: write fields in separate components#407

Open
germasch wants to merge 34 commits into
psc-code:mainfrom
germasch:pr/writer_adios2
Open

WriterADIOS2: write fields in separate components#407
germasch wants to merge 34 commits into
psc-code:mainfrom
germasch:pr/writer_adios2

Conversation

@germasch

@germasch germasch commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@germasch germasch force-pushed the pr/writer_adios2 branch from fbce9b0 to f036bf4 Compare June 5, 2026 00:45
@germasch germasch force-pushed the pr/writer_adios2 branch from febdc61 to d4df79b Compare June 5, 2026 23:25
@germasch germasch force-pushed the pr/writer_adios2 branch from 8cd8993 to ad54d44 Compare June 7, 2026 14:11
@germasch germasch marked this pull request as ready for review June 8, 2026 01:46
@JamesMcClung

Copy link
Copy Markdown
Collaborator

Thanks! This and psc-code/pscpy#42 works perfectly in psc-plot, but they have the effect of changing "t" to "time". I'd personally prefer "t" as the dimension and scalar value, but I can manually rename downstream if you disagree.

Also, what are the reasons for storing dimensions in a single space-separated string? Just seems odd.

@JamesMcClung

Copy link
Copy Markdown
Collaborator

Also, shouldn't it be "step/dimensions", not "step_dimension"?

@germasch

Copy link
Copy Markdown
Contributor Author

Thanks! This and psc-code/pscpy#42 works perfectly in psc-plot, but they have the effect of changing "t" to "time". I'd personally prefer "t" as the dimension and scalar value, but I can manually rename downstream if you disagree.

I think that should be easy enough to change (by changing step_dimension to t). Since we just use short symbols (x, y, z) otherwise, I guess it makes sense to do that here, too. To stay consistent, it means the actual time variable (coordinate) also needs to change, though, but it can have a long_name attribute set to time.

Also, what are the reasons for storing dimensions in a single space-separated string? Just seems odd.

Well, I was going to say that I copied this from how netcdf4 does it, but actually that may not be true. I'm also not totally positive that adios2 can handle an attribute that's an array of strings, but I'll take a look.

@germasch

Copy link
Copy Markdown
Contributor Author

Also, shouldn't it be "step/dimensions", not "step_dimension"?

Well, what I had in mind is that adios2's concept of steps should translate into an additional dimension in xarray, and that attributes to serve to give it a name (other than step, which I think is default). So it's not quite the same as the usual var/dimensions, since it's purely a dimension, not specific to any particular variable. I somewhat agree that your suggestion reads better, though, but this is also already used in openggcm, so anything that changes here needs changes there, too, plus potential compatibilities woes (though I guess I might not actually care enough).

@JamesMcClung

Copy link
Copy Markdown
Collaborator

Well, what I had in mind is that adios2's concept of steps should translate into an additional dimension in xarray, and that attributes to serve to give it a name (other than step, which I think is default). So it's not quite the same as the usual var/dimensions, since it's purely a dimension, not specific to any particular variable.

That kinda makes sense to me, but the actual implementation doesn't seem to match that. Here's a snippet of the current output:

double   jz_ec               {16, 128, 1} = 0 / 0
string   jz_ec/dimensions    attr   = "z y x"
int32_t  step                attr   = 12
string   step_dimension      attr   = "time"
double   time                scalar = 0.397748
double   x                   {1} = 0.03125 / 0.03125
string   x/dimensions        attr   = "x"

To match how x handles its own dimensions, I think step should be like this:

int32_t  step                attr   = 12
string   step/dimensions     attr   = "step"

And similarly, time would be:

int32_t  t                scalar  = 0.397748
string   t/dimensions     attr    = "t"

I don't know about scalar vs. attr vs. length-1 1D array, or if maybe step/dimensions should be "t" like it currently is.

this is also already used in openggcm, so anything that changes here needs changes there, too, plus potential compatibilities woes (though I guess I might not actually care enough).

Assuming this impacts xarray_adios2, can ggcmpy handle the discrepancy?

@germasch

Copy link
Copy Markdown
Contributor Author

Well, so if one were to use step/dimension = step, then data would become associated with (x,y,z,step), and step the default coordinate array for it, but not a particularly useful one. I guess one would also get a time array with step dimensions, but I think it wouldn't even be considered a coordinate by default. I guess one could make it so, and xarray can have multiple coordinates associated with the same dimension, but to me this all seems more confusing than need be.

Also, step could end up kinda overloaded (well, it already is): It could refer to the adios2 step we're in, or to the PSC timestep number. I know step_dimension is an exception / special treatment, but that kinda corresponds to adios2's special handling of steps.

I guess I didn't even remember that psc writes out a step variable (well, actually, it doesn't, it writes it as an attribute, which means it won't turn into a xarray variable, anyway). It could write a step variable, and that would become a coordinate, and one could even turn that into the default coordinate (by having step_dimension == "step" or not present). The existence of step_dimension actually makes step not special in this output format in general. It allows the writer (like PSC) to specify the additional dim/coordinate -- and I'd still be in favor of choosing t for PSC.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants