Cast GDP float64 data to float32 as an option (default)

[milancurcic]

This casts all float64 variables to float32 before returning the Dataset in the RaggedArray.to_xarray() method, but give an option to not do it if desired. I tested it with a small gdp-2.01 file (10 random drifters) and it works.

Can you confirm that float32 is OK for longitude and latitude variables? We get precision of 7 digits. For a drifter in the 3-digit longitudes, precision is down to O(10 m). Is it sufficient?

[selipot]

Good to go for me for now. A user who would want to keep some variables float64 would have to pass False and do the conversion manually. I think this is acceptable.

[selipot]

I was able to discuss this a bit more with @JeffreyEarly : we should keep double precision for latitude and longitude estimates but we can convert to single precision for all other variables. (Even for uncertainty of longitude and latitude.)

Since this PR applies to the whole raggedarray class I think it is not a good idea in the end. We should keep double precision. and manually convert what we need to convert for the GDP dataset.

[milancurcic]

Sounds good, we can exclude specific variables from the cast. Do you know why double for velocity?

[selipot]

Sounds good, we can exclude specific variables from the cast. Do you know why double for velocity?

That was a typo: latitude and longitude, now corrected.

[selipot]

So in conclusion, despite my commit, I don't think we should implement this PR.

[milancurcic]

I'm confused. Why not cast variables other than lat and lon?

[selipot]

I'm confused. Why not cast variables other than lat and lon?

If I understand correctly, this PR proposes to convert all variables to float32 (not only in the GDP cases!). Some users might want to retain the double precision for their specific variables. So I am not in favor of this behavior.

[milancurcic]

OK, if you prefer manually casting that's fine. Better than adding code that won't be useful.

As an aside as since it came up here, the RaggedArray class is currently only used for GDP. I can't imagine using it for other datasets over simple functions like I did in MOSAiC. Long term I'd like to remove it altogether; nothing to do for now.

[philippemiron]

I'm confused by this. So we are doing nothing and the GDP ragged array will be 35 GB? I don't think it makes much sense to have all this precision.

[selipot]

26GB not 35GB. We are doing the conversion by hand for the GDP, not by a default behavior of raggedarray.to_netcdf(). The manual conversion leads to ~16GB.

[philippemiron]

But it could be done in adapters.gdp1h and adapters.gdp6h so we don't have to do anything manually (and limit possible divergence) when regenerating the GDP ragged array.

[selipot]

I think that's a great idea @philippemiron

[milancurcic]

It sounds like the current direction is:

• Move casting to adapters.gdp1h and adapters.gdp6h
• Exclude time, latitude, and longitude from the cast

Please confirm.

[selipot]

Sounds like a plan.

[selipot]

Is this where you should add variables_to_skip = ["lon", "lat", "time"]?`

[milancurcic]

That's the default, see the function definition.

[selipot]

Once this is done I will be able to get the final parquet file.

[milancurcic]

@selipot Can this be merged?

[selipot]

yes!