Skip to content

Conversation

@mandli
Copy link
Member

@mandli mandli commented Sep 8, 2025

Addresses #656.

@rjleveque
Copy link
Member

I think you also need to modify the Fortran code gauges_module.f90 so that the meaning of 2 and 3 are swapped, e.g. around line 295 and line 759.

And similarly in amrclaw in clawpack/amrclaw#309.

I note also there's a separate src/2d/shallow/multilayer/gauges_module.f90 but this doesn't seem to support binary output at all?

Otherwise this seems fine, but we should add something in the release notes pointing out that if a user actually used an integer 2 or 3 in their setrun.py to specify this, they may want to adjust it. If they specified 'binary32' or 'binary64` it should still work as expected as long as the Python and Fortran code are consistent with each other.

@mandli
Copy link
Member Author

mandli commented Sep 9, 2025

I think the changes in gauges_module.f90 is correct, but double check. I also just slightly modified the error code checking for valid file_formats.

The multilayer code does not have binary, and neither does AMRClaw, which is why I did not modify it. I did raise an issue in AMRClaw to do so (clawpack/amrclaw#308), but I can do the same for multilayer output.

I was wondering if we wanted to add a warning in the Python saying that these were switched, or if we just put it in the documentation/release notes. We should do something before releasing or merging.

@rjleveque
Copy link
Member

I'm confused at the moment. With this new version, if I set

   rundata.gaugedata.file_format = 'binary32'

in setrun.py and do make data then gauges.data contains:

# File format (1=ascii, 2=binary64, 3=binary32)
3 

which is the old value 3 rather than 2. This makes sense since this file is written by amrclaw/src/python/amrclaw/data.py which hasn't been modified and probably needs to be.

But then when I run the code the resulting gaugeNNNNN.txt file has a header that contains

# file format binary32, time series in .bin file

even though gauges_module.f90 was modified, so shouldn't it be writing out binary64 since it reads file_format == 3 from gauges.data? Also the resulting bin file is smaller by a factor 2 than what I get when I specify binary64, so it seems to do what's desired but I don't understand why.

@rjleveque
Copy link
Member

rjleveque commented Sep 10, 2025

Sorry, I forgot there was clawpack/amrclaw#309 which fixes amrclaw/src/python/amrclaw/data.py, I'll take another look at this...

@rjleveque
Copy link
Member

Looks good now, thanks!

@rjleveque rjleveque merged commit 93293b2 into clawpack:master Sep 10, 2025
1 check failed
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