JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
9 stars 5 forks source link

Feature/nicke air bc record variable #1568

Closed nicholasesposito closed 2 weeks ago

nicholasesposito commented 1 month ago

Description

This PR switches the aircraft bias converter script so the output dimensions are [Record, Variable] rather than [Variable, Record]. This is the style agreed at this discussion () and it fixes some tests failures with debug builds which don't collapse the array dimensions.

Issue(s) addressed

Resolves UFO#3480 and aids UFO#3499 Resolves ioda-conv #1567

Dependencies

List the other PRs that this PR is dependent on:

Impact

Expected impact on downstream repositories: None?

Checklist

BenjaminRuston commented 3 weeks ago

thanks @nicholasesposito this seemed to work for me. Before and after your change two output files:

DIFFER : VARIABLE : numberObservationsUsed : DIMENSION : Record <> Variable
DIFFER : VARIABLE : numberObservationsUsed : DIMENSION : Variable <> Record
BenjaminRuston commented 3 weeks ago

@CoryMartin-NOAA or @danholdaway think this is nothing to be concerned with but basically I ran before the PR on a aircraft bias file I pulled from AWS S3, then compiled PR ran again and did a nccmp on the two results.

showed the dimension flip but also said:

-0700 ERROR /Users/benr/work/JCSDA/JEDI/spack-stack-1.8.0/cache/build_stage/spack-stage-nccmp-1.9.0.1-cshxkvpc3inee4zytkc367pqe6ai3tow/spack-src/src/nccmp_data.c:3450 NetCDF: Start+count exceeds dimension bound

thinking may be due to it getting confused as the files have flipped dimensions.. as the ctest using nccmp seems file

nicholasesposito commented 3 weeks ago

@CoryMartin-NOAA or @danholdaway think this is nothing to be concerned with but basically I ran before the PR on a aircraft bias file I pulled from AWS S3, then compiled PR ran again and did a nccmp on the two results.

showed the dimension flip but also said:

-0700 ERROR /Users/benr/work/JCSDA/JEDI/spack-stack-1.8.0/cache/build_stage/spack-stage-nccmp-1.9.0.1-cshxkvpc3inee4zytkc367pqe6ai3tow/spack-src/src/nccmp_data.c:3450 NetCDF: Start+count exceeds dimension bound

thinking may be due to it getting confused as the files have flipped dimensions.. as the ctest using nccmp seems file

I was getting something similar when I was comparing to the wrong files because I did not update to the right UFO-data branch. Are you using the right UFO-data branch?

BenjaminRuston commented 3 weeks ago

in short @nicholasesposito think you're verifying what I'm saying, that nccmp has trouble comparing two files before and after this change

the ufo-data branch is out of play here as I took a full file I pulled from AWS:

aws s3 cp s3://noaa-gfs-bdp-pds/gdas.20201214/18/gdas.t18z.abias_air .

ran the converter by hand (not in the ctest) from develop to produce an output:

${JEDI_SRC}/build/bin/acftbias2ioda.x acftbias_converter.yaml 

then made your branch ran again and compared the two outputs. If you saw something similar comparing a new output to an old output then we both got similar output from nccmp .. sounds like a good thing

also just interrogating the output from the converter there appears to be nothing anomalous with the netCDF file so approved the PR, and think we're ready to move this forward

BenjaminRuston commented 3 weeks ago

@ADCollard are you using this in your GDASApp already, could you have a look and review