Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ncap2 convert out-of-range integer input to double precision #118

Open
czender opened this issue Mar 7, 2019 · 2 comments
Open

ncap2 convert out-of-range integer input to double precision #118

czender opened this issue Mar 7, 2019 · 2 comments
Assignees

Comments

@czender
Copy link
Member

czender commented Mar 7, 2019

Please test for out-of-range integers on the RHS and convert them to double-precision instead of silently wrapping them into negative numbers. Current and desired behavior:

zender@firn:~$      ncap2 -O -v -s 'foo=289788500'  ~/nco/data/in.nc ~/foo.nc # works
zender@firn:~$      ncks -v foo ~/foo.nc
netcdf foo {
variables:
  int foo ;

data:
  foo = 289788500 ;

} // group /
zender@firn:~$      ncap2 -O -v -s 'foo=28978850000' ~/nco/data/in.nc ~/foo.nc # borken, should produce type double instead
zender@firn:~$      ncks -v foo ~/foo.nc
netcdf foo {
variables:
  int foo ;

data:
  foo = -1085921072 ;

} // group /
zender@firn:~$ 
czender added a commit that referenced this issue Dec 18, 2019
…from Daniel Macks). ncclimo global average timeseries working prototype with --glb_avg.
@rkouznetsov
Copy link
Contributor

I agree that wrapping is rarely a desired effect. Silent type conversion is also a nasty feature. I would prefer it to crash on such occasion and request user to explicitly specify it with "double(..)" or just by adding a period after the number. If a person wants to fit such a number into integer, they better know what they are doing.
Also, consider a case when wrapping occurs as a result of some arithmetic. Then again, capturing wrapping could save hours of debugging...

@hmb1
Copy link
Contributor

hmb1 commented Aug 4, 2020

@czender

I have had a good look at the grammar.
The issue I have with converting an out of range integer to a NC_DOUBLE is that I will have to have to perform the type conversion twice. on the first parse and again on the second parse. Otherwise there will be a conflict between say an NC_INT on the first parse and say a NC_DOUBLE on the second parse. I don't see the need of adding this overhead for such a rarely ocuring event.

I have added range checking for all non-float number conversions.
If the number is out of range then an error message is emitted and ncap2 terminates.
This may break some scripts !! Especially some with ram variables !!
Will add to repo tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants