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

F90 module list generation #646

Closed
wants to merge 2 commits into from
Closed

Conversation

IvanaEscobar
Copy link
Contributor

@IvanaEscobar IvanaEscobar commented Jul 29, 2022

What changes does this PR introduce?

Bug in f90mkdepend tool.

What is the current behaviour?

modreflist keeps a carriage return character if modules are loaded with a line break. This affects any code that has USE <modulename> type integration

Example module loading scenario for blabla_do_something.F90:

#include "BLABLA_OPTIONS.h"
...
    USE blablareadin,   only: read_some_stuff, blabla_var
    USE blablaphysics
...

f90mkdepend produces the following warning to error output:

WARNING: f90mkdepend: no source file found for module blablaphysics

and f90mkdepend.log (in vim) produces:

blabla_do_something.F90 => blablareadin
blablaphysics^M

This creates missing dependencies for blabla_do_something.f90, which leads to compilation errors.

What is the new behaviour

Updated f90mkdepend tool accounts for carriage returns when creating modreflist. Now ^M is removed from module name blablaphysics

Does this PR introduce a breaking change?

Not that I know of

Other information:

Tested using a package in development written in F90, with the current mitgcm code checkpoint, and on the verification problem tutorial_global_oce_latlon

Suggested addition to tag-index

@IvanaEscobar
Copy link
Contributor Author

IvanaEscobar commented Aug 29, 2022

sample f90 package

sample file: line 14 produces a WARNING from f90mkdepend
USE iHopMod

sample f90mkdepend output
f90mkdepend.log
f90mkdepend.err.txt

Checked on:
Linux with gfortran: Makefile_gfortran.txt
Linux with ifort: Makefile_ifort.txt
Linux with ifort+mpi Makefile_ifort+mpi.txt

@jrscott
Copy link
Member

jrscott commented Aug 29, 2022

Thanks @IvanaEscobar

@jahn if you have a minute, could you take a look at the problem and solution here, want to make sure you are on board. Thanks

@jahn
Copy link
Member

jahn commented Aug 29, 2022

Thanks, Ivana, for pointing these out.

We generally try not to have carriage returns in source files, so the first error doesn't occur.

For compiled modules, the convention in MITgcm is that a module mymodule has to be defined in a source file of one of the following names:

mymodule.F90
mymodule.F
mymodule_mod.F90
mymodule_mod.F

If the module is precompiled, dependency resolution is not an issue and the warning can be ignored.

@IvanaEscobar
Copy link
Contributor Author

@jahn

Line 14 of the sample file doesn't have a carriage return included. So it is unclear when a carriage return is introduced. Maybe an artifact of using awk parsing?

The sample f90 package contains the module specified in a source file written in the style you pointed out. Specifically as ihopmod.F90 (link to file).

I don't change the inner workings of genmake2 which links the sample file to each of the modules it specifies as shown in Makefiles in the comment above (Makefile_ifort.txt)

If it's easier for me to demo the bug for you, I can illustrate the compilation warnings via Zoom?

@jahn
Copy link
Member

jahn commented Aug 30, 2022

I cloned your repository and removed all carriage returns from the files and f90mkdepend works fine. I used sed -i 's/\r//g' * in the ihop directory but you could also use dos2unix.

@IvanaEscobar
Copy link
Contributor Author

I was able to reproduce no warnings with your fix to the package.

Vim wasn't displaying carriage returns for FORTRAN type files. So the bug is fixed by clearing return carriages from the source files.

@IvanaEscobar
Copy link
Contributor Author

@jahn I was wondering what happens if you do not have write permissions to fix a file using sed

This happens in our Linux cluster with the netcdf header file. There are instances in that header file where they use multi-line comments that happened to produce lines that start with the word use

/* comment starts .....
use this feature ....
more comment ....
and done */

with the current f90mkdepend, this comment will trigger a warning.
WARNING: f90mkdepend: for netcdf.h no source file found for this

Overall, it's harmless but was curious on potential solutions

@owang01
Copy link
Collaborator

owang01 commented Apr 26, 2023

@IvanaEscobar See if changing lines 17-19 of f90mkdepend as the following would get rid of the warning

if grep -i '^ *use ' $filename | grep -iv '^ *use this' $filename > /dev/null; then
   # extract module name in lower case
   modreflist=$(grep -i '^ *use ' $filename | grep -iv '^ *use this' | awk '{print tolower($2)}' | sed 's/,.*$//') 

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

Successfully merging this pull request may close these issues.

None yet

4 participants