NOAA-GFDL / FRE-NCtools

Tools for manipulating and creating netCDF inputs for FMS managed models
GNU Lesser General Public License v3.0
20 stars 28 forks source link

mppnccombine needs update to use the input format by default #319

Open ceblanton opened 1 month ago

ceblanton commented 1 month ago

Describe the bug mppnccombine supports Netcdf-3 classic, Netcdf-3 64-bit offset, and Netcdf-4 input file and output file formats.

The FRE team believed that if you specify the output format (i.e. -64 or -n4), then the tool will use the format. Otherwise, it should use the input format.

Unfortunately, this is not how the tool works. How it works is: If you specify the output format (-64 or -n4), then the tool will use that format (Netcdf-3 64-bit offset and Netcdf-4 classic). But if the output format is not specified, then the default is Netcdf-3 classic.

To Reproduce Find some test input files and convert them so both Netcdf-4 and Netcdf-3 64-bit offset are available.

Run mppncombine without specifying the output format, and observe the behavior.

In both cases, the tool chooses the never-desired Netcdf-3 classic format.

(use ncdump -k to see the file type)

Expected behavior When the input files are either Netcdf-3 64-bit offset or regular Netcdf-3, and no output format is specified, then the output format should be Netcdf-3 64-bit offset.

When the input files are Netcdf-4 and no output format is specified, then the output format should be Netcdf-4.

System Environment any

Additional context Add any other context about the problem. If applicable, include where any files that help describe, or reproduce the problem exist.

ceblanton commented 1 month ago

@HansVahlenkamp, as the original and most recent mppnccombine maintainer, could you fix this? Thank you!

mlee03 commented 1 week ago

Hi @HansVahlenkamp, any update on this issue? :grin:

HansVahlenkamp commented 1 week ago

Distracted with some other issues that needed attention. Will work on this next next week.

mlee03 commented 1 week ago

@HansVahlenkamp, got it. I'll pester you next week then :)

ceblanton commented 1 week ago

@HansVahlenkamp , when you update mppnccombine, please come up with 3 clear test cases to show that the fix is good.

@underwoo volunteered to convert your documented test cases to unit tests, but he needs the 3 clear input files to do this.

Thank you! (and PS, I'll be pestering you too! we really need this update next week)

Test case 1: Netcdf-3 (not 64-bit) input: output should be Netcdf-3 64-bit

Test case 2: Netcdf-3 (64-bit) input: output should be Netcdf-3 64-bit

Test case 3: Netcdf-4 input: Netcdf-4 output

mlee03 commented 5 days ago

@HansVahlenkamp, just checking, any updates?

HansVahlenkamp commented 4 days ago

@mlee03 I'm working on it this week and hope to have an updated version by next week...

Starting with version 2.2, there were significant structural changes done to the code by another author. The first step is reviewing those changes compared to the last version that I wrote. Also, my NetCDF test input files are quite old at this point and I could use some new ones (which are not too large) that represent all three of the test cases described by @ceblanton. Do you have any available?

ceblanton commented 4 days ago

@HansVahlenkamp I'm sure your old test files are OK but I'll dig up some new ones.

ceblanton commented 4 days ago

@HansVahlenkamp here are some distributed files on gaea, in the 3 formats:

gaea57:~%>ls /gpfs/f5/scratch/gfdl_f/Chris.Blanton/test-files/*                                                                                                                                                                                   1079
/gpfs/f5/scratch/gfdl_f/Chris.Blanton/test-files/nc3:
out.nc  vegn2.res.tile6.nc.0000  vegn2.res.tile6.nc.0001  vegn2.res.tile6.nc.0002  vegn2.res.tile6.nc.0003  vegn2.res.tile6.nc.0004

/gpfs/f5/scratch/gfdl_f/Chris.Blanton/test-files/nc3-64:
out.nc  vegn2.res.tile6.nc.0000  vegn2.res.tile6.nc.0001  vegn2.res.tile6.nc.0002  vegn2.res.tile6.nc.0003  vegn2.res.tile6.nc.0004

/gpfs/f5/scratch/gfdl_f/Chris.Blanton/test-files/nc4:
out.nc  vegn2.res.tile6.nc.0000  vegn2.res.tile6.nc.0001  vegn2.res.tile6.nc.0002  vegn2.res.tile6.nc.0003  vegn2.res.tile6.nc.0004
mlee03 commented 1 hour ago

Hi @HansVahlenkamp, any chance the PR can be made in the next couple of days?