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

Love number parameter array exceeds Fortran line limit #504

Open
marshallward opened this issue Oct 13, 2023 · 3 comments
Open

Love number parameter array exceeds Fortran line limit #504

marshallward opened this issue Oct 13, 2023 · 3 comments

Comments

@marshallward
Copy link
Member

marshallward commented Oct 13, 2023

The Love_data parameter in MOM_load_Love_numbers.F90 is split over ~1440 lines. A standard-compliant compiler is only required to support up to 255 lines. Although not generally a problem, this is preventing us from enabling the -pedantic flag for testing.

Unfortunately, I cannot think of any way to split a parameter across multiple lines. Even if I could, I am not sure if it's in our interest to have these numbers hard-coded into the executable.

We could change the array from a parameter to a variable, and fill in the records. We could also store the numbers as an input file, although I think this would be the first instance of such a large dataset added to the codebase. We don't use namelists very much, but this might be a good use of one.

@herrwang0 We discussed this problem back when it was first merged. Do you have any ideas or thoughts on how we might handle this array?

@herrwang0
Copy link

I didn't like how this was handled either, as I don't think these numbers (probably any parameter) should be hard-coded.

It feels to me the nature of this data array is somewhere between an input file and a parameter. It is too large to be an entry in MOM_input. But it is also just a set of constants that does not feel right to be a user-specified input file.

I had the same idea of including the array as a file as part of the codebase and got version-controlled, but there was no precedence. So I just quarantined it into a separate module as the next best thing...

@herrwang0
Copy link

In fact, the same may apply to tides related parameters (frequencies, amplitudes, Love numbers, astronomical arguments etc). Currently these are input parameters that can be changed (as they should be, for fun experiments and tests), but the defaults are hard-coded.

These tidal parameters may get expanded in the future (more constituents, various versions of astronomical arguments), which means larger hard-coded parameter arrays.

@marshallward
Copy link
Member Author

I am leaning towards moving the numbers into a namelist. I see a few advantages:

  • Relatively easy to read without tools

  • Text format, easy for git to track, as opposed to a binary netCDF file

  • Users can replace it with their own namelist file if needed

Some downsides:

  • We have generally avoided using namelists up to now, with the FMS input.nml being the lone exception.

  • Users may not find it easy to create their own namelist, although there are options (e.g. f90nml).

  • Parsing the values-as-strings could be ambiguous if not done properly. A binary netCDF would be unambigous. (aAthough I don't know if a namelist is any worse than writing the numbers in a Fortran file.)

  • A binary-packed netCDF file is likely to be smaller (although we're talking some fraction of 100kB in ~10MB of source).

I guess another alternative is to see if the MOM_input parser can handle arrays like this, and perhaps it as a the default file, which can potentially be overridden. Interested to know what others think.

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

2 participants