FMS
FMS copied to clipboard
Environmental vars use_LARGEFILE and MAXFIELDMETHODS should be set by configure, not on command line
The Travis tests do this:
export CPPFLAGS="-I/usr/include -Duse_LARGEFILE -DMAXFIELDMETHODS_=500"
We don't want to set important variables on the command line. That is impossibly opaque to users.
Instead, we set them with configure. We can do this by choosing a default value, and letting the user over-ride the default if they want. That way, users get a useful build without pouring over the documents and setting the correct magic variables, and advanced users still retain full control over the build.
For these two I we add two new enable/disable options to configure.
If the default is to use_LARGEFILE then we have an option --disable-largefile which allows the user to turn it off.
If a good default for MAXFIELDMETHODS is 500, we set that in configure, and provide an option --enable-maxfieldmethods=N which the user can use to set the value to N instead of 500. (Also what is the max and min maxfieldmethods? We can have configure to check to ensure a valid value is selected, if the default is overridden.)
This also allows us to have some output in the configure which reflects the chosen values. This kind of information becomes invaluable when debugging user problems. Instead of asking the user to tell you (and possibly get it wrong) you have the configure output that expressly includes this information. When a user has a problem you tell them to send you the output of configure and the answers will be there.
Then we change the .travis.yml file to test using the configure options instead of using the environment vars.
Also, what is the purpose of use_LARGEFILE?
I see that it is used only in mpp/include/mpp_io_connect.inc:
find . -name '*'|xargs grep -s use_LARGEFILE
./mpp/include/mpp_io_connect.inc:#elif use_LARGEFILE
When I look there, I see that use_LARGEFILE seems to indicate that 64bit_offset format be used for the netCDF files.
You may consider adding support for CDF5, a format similar to 64-bit offset, but without the size limitations. Or NC_NETCDF4, which would also allow you to use compression transparently.
Is this really something you need to set at build time? It would seem more sensible to make this a run-time parameter. If you are going to set it at configure, may I suggest a more specific name.
Investigating MAXFIELDMETHODS_ I see:
(base) ed@mikado:~/FMS$ find . -name '*'|xargs grep -s MAXFIELDMETHODS
./field_manager/field_manager.F90:#ifndef MAXFIELDMETHODS_
./field_manager/field_manager.F90:#define MAXFIELDMETHODS_ 250
./field_manager/field_manager.F90:integer, parameter :: MAX_FIELD_METHODS = MAXFIELDMETHODS_
So MAXFIELDMETHODS_ sets the Fortran parameter MAX_FIELD_METHODS, which is then used in many places.
There is no need to codify changing MAX_FIELD_METHODS. It getting added to the test is a legacy item. The new GitHub actions version does not include this. In most cases, if MAX_FIELD_METHODS needs to be increased, we will increase it in the code. This macro is a way to allow tests to be done on large diag_table files.
The use_LARGEFILE may be something to enable via configure. We should check again if this is really needed anymore (we will remove the use_netCDF macro requirement soon).
Closing this since the LARGE_FILE is set by configure now, we don't set any CPPFLAGS in our current unit testing.