Incorporate support for new CDF-5 external data types in SCORPIO

E3SM-Project / scorpio

A high-level Parallel I/O Library for structured grid applications

Other

21 stars 16 forks source link

Incorporate support for new CDF-5 external data types in SCORPIO #525

Closed bartgol closed 10 months ago

bartgol commented 1 year ago

As an extension of CDF-2, CDF-5 format has additional external data types, namely NC_UBYTE, NC_USHORT, NC_UINT, NC_INT64, and NC_UINT64.

This PR includes support for these new CDF-5 data types in SCORPIO, across all calls and IO types. Additionally, the C unit tests have been updated to include tests for these types.

bartgol commented 1 year ago

@jayeshkrishna @dqwu I'm going through PRs/issues I have open, and found this one. Any thought of integrating it?

jayeshkrishna commented 1 year ago

@dqwu : There are some things to add to this PR before merging it,

Ensure that NC_INT64 support is available for all calls/IO types
Add support for NC_UINT64
Ensure that the tests are updated to test these types
Add a configure check that long long ints are 64-bit

@dqwu: You can create a new branch off master, cherry pick the commit in this PR and add further commits to fix the issues above

dqwu commented 11 months ago

@jayeshkrishna Please re-review this updated PR, thanks.

jayeshkrishna commented 11 months ago

We would also need the configure checks for ensuring "long long"s are 64-bit for example. If not, we need to use a 64-bit datatype to write the data out

jayeshkrishna commented 11 months ago

Also, we will need to add support for the corresponding Fortran types (NF_*)

dqwu commented 11 months ago

We would also need the configure checks for ensuring "long long"s are 64-bit for example. If not, we need to use a 64-bit datatype to write the data out

We already have this configure check in this PR. Also, PnetCDF uses "longlong" for API names and C type "long long" for API arguments to support 64-bit data types.

dqwu commented 11 months ago

Also, we will need to add support for the corresponding Fortran types (NF_*)

I think we can support that in another PR. Also note that existing Fortran unit tests only use PIO_TF_DATA_TYPE = "PIO_int" and PIO_TF_FC_DATA_TYPE = "integer" (no 64-bit int types).

jayeshkrishna commented 11 months ago

We would also need the configure checks for ensuring "long long"s are 64-bit for example. If not, we need to use a 64-bit datatype to write the data out

We already have this configure check in this PR. Also, PnetCDF uses "longlong" for API names and C type "long long" for API arguments to support 64-bit data types.

Ok, at least the way I see it the NetCDF type has a fixed size while a C long long does not (although it has a min size). I just don't want the configure to fail (on a new system/compiler) and end up in a situation where we have not way to disable it

jayeshkrishna commented 11 months ago

Also, we will need to add support for the corresponding Fortran types (NF_*)

I think we can support that in another PR. Also note that existing Fortran unit tests only use PIO_TF_DATA_TYPE = "PIO_int" and PIO_TF_FC_DATA_TYPE = "integer" (no 64-bit int types).

Ok, without this fix only the C interface has access to the new types (and we might drop/forget adding support for the Fortran types when we get busy with other issues). The unit testing framework can be expanded to cover more types, right?