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

netCDF 4.8.0 ncgen breaks on CDL type "long"? #1977

Open
czender opened this issue Apr 7, 2021 · 12 comments
Open

netCDF 4.8.0 ncgen breaks on CDL type "long"? #1977

czender opened this issue Apr 7, 2021 · 12 comments

Comments

@czender
Copy link
Contributor

czender commented Apr 7, 2021

netCDF 4.8.0 appears to break NCO builds because ncgen no longer supports the type "long" as a synonym for "int" in CDL files. I did not notice this backwards-incompatibility in the 4.8.0 release notes. Is it intentional?

With latest snapshot:

zender@glace:~/nco/data$ ncgen -v
ncgen: option requires an argument -- 'v'
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [-W generate whole var upload] [file ... ]
netcdf library version 4.8.1-development of Mar 30 2021 16:12:40 $
zender@glace:~/nco/data$ ncgen -o in.nc in.cdl
Undefined name (line 642): long
zender@glace:~/nco/data$ 

With 4.7.4 and earlier:

zender@sastrugi:~/nco/data$ ncgen -v
ncgen: option requires an argument -- v
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-B buffersize] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [file ... ]
netcdf library version 4.7.4 of Dec 16 2020 23:04:20 $
zender@sastrugi:~/nco/data$ ncgen -o in.nc in.cdl
zender@sastrugi:~/nco/data$ 

Input file in.cdl is here: https://github.com/nco/nco/blob/master/data/in.cdl

@DennisHeimbigner
Copy link
Collaborator

Did not mean to do this. Not sure how it happened. But I will check it out
and see if there was a reason for it.

@DennisHeimbigner
Copy link
Collaborator

This really bizarre.
I did the in.cdl test in my local master and it worked ok.
I did the same test in the netcdf-c-4.8.0 distribution and it failed as you describe.
I diffed the ncgen code for the two trees and they are identical.

I have no idea what is going on.

@edwardhartnett
Copy link
Contributor

Happening in the C library instead of ncgen?

@czender
Copy link
Contributor Author

czender commented Apr 7, 2021

Thanks for looking into this. Sorry, I had to modify that file to replace the CDL "long" keyword with "int" in order for NCO to build with the new 4.8.0 netCDF libraries used by Azure. Here is a smaller test file:

netcdf in {
variables:
long long_var;
data:
long_var=10;
}

@DennisHeimbigner
Copy link
Collaborator

I built both versions using the same set of options on the same os (ubuntu 16)
which means same tool chain.
At the moment, the only thing I can think of is something textual where building the distribution introduced, say, some '\r' character.

@DennisHeimbigner
Copy link
Collaborator

But since I have repeated the error, hopefully I can debug it.

@opoplawski
Copy link
Contributor

@czender I take it that there is no test in nco for this? The nco Fedora package seems to build fine with netcdf 4.8.0 in my testing.

@czender
Copy link
Contributor Author

czender commented Apr 11, 2021

Thanks for looking into this @opoplawski. The Fedora build does attempt to create a netCDF4 version of in.cdl and the build.log shows normal behavior when converting:

...
/usr/bin/ncgen -k netCDF-4 -o in_4.nc in.cdl 
make[2]: Leaving directory '/builddir/build/BUILD/nco-4.9.8/data'
Making all in src
...

So I went back and tested ncgen 4.8.0 with all possible binary output formats, and the results are...surprising?:
This script (which anyone should be able to replicate by pasting in a window)

cat > ~/in.cdl << 'EOF'
netcdf in {
  variables:
    long long_var;
  data:
    long_var=10;
}
EOF
ncgen -v
ncgen -o ~/in.nc ~/in.cdl
ncgen -3 -o ~/in.nc ~/in.cdl
ncgen -4 -o ~/in.nc ~/in.cdl
ncgen -5 -o ~/in.nc ~/in.cdl
ncgen -6 -o ~/in.nc ~/in.cdl
ncgen -7 -o ~/in.nc ~/in.cdl

leads to these results:

zender@glace:~$ cat > ~/in.cdl << 'EOF'
> netcdf in {
>   variables:
>     long long_var;
>   data:
>     long_var=10;
> }
> EOF
zender@glace:~$ ncgen -v
ncgen: option requires an argument -- 'v'
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [-W generate whole var upload] [file ... ]
netcdf library version 4.8.1-development of Mar 30 2021 16:12:40 $
zender@glace:~$ ncgen -o ~/in.nc ~/in.cdl
Undefined name (line 3): long
zender@glace:~$ ncgen -3 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -4 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -5 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -6 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -7 -o ~/in.nc ~/in.cdl
zender@glace:~$ 

So ncgen only breaks (for me) when it gets no binary format option. Hopefully this clue will help @DennisHeimbigner isolate the problem.

@DennisHeimbigner
Copy link
Collaborator

I have a solution, I just have not had time to put up the PR. I will try to get to it
next.

DennisHeimbigner added a commit to DennisHeimbigner/netcdf-c that referenced this issue Apr 13, 2021
re: Unidata#1977

PR Unidata#1753, changed ncgen
to allows certain type names to be used as identifiers in
selected situations.

An unwanted side effect was that existing type aliases no longer
were accepted by ncgen. Specifically, using the "long" type
caused an error.

I was able to figure out a better solution to the original
problem (Unidata#1750)
that also fixes this problem as well.

This PR fixes that problem in ncgen/ncgen.l,
and adds tests to ncdump/test_keywords.sh
@DennisHeimbigner
Copy link
Collaborator

See PR #1984

@czender
Copy link
Contributor Author

czender commented Apr 14, 2021

Thanks @DennisHeimbigner. I thought the fix might be a one-liner, that PR shows how little I know!

@DennisHeimbigner
Copy link
Collaborator

Ideally it should have been a 1-liner. But in my endless pursuit for complexity,
I avoided that :-)

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

No branches or pull requests

4 participants