wimvanderbauwhede / RefactorF4Acc

An Automated Fortran Code Refactoring Tool to Facilitate Acceleration of Numerical Simulations
Other
57 stars 9 forks source link

Short example #6

Closed rouson closed 5 years ago

rouson commented 5 years ago

It would be instructive to provide an example that succinctly demonstrates as many of the capabilities in the Features list as is feasible in 50 to 100 lines of code.

wimvanderbauwhede commented 5 years ago

There is an example of the 2-D Shallow Water equation in the tests folder, does that not meet your requirements? It is 180 lines of code in total. Although this code has .f95 extensions, it is actually F77 code with F95 type declarations. It is taken from the book "Ocean Modelling for Beginners" by Kaempf.

The only feature this example does not demonstrate is GOTO conversion, but there are plenty of these in the NIST testbench. I have added a README.md.

rouson commented 5 years ago

Here's what I get when I try to refactor this example after installing scons:

$ ./generate_and_build.sh 
scons: Reading SConscript files ...
IndexError: list index out of range:
  File "/Users/rouson/Code/RefactorF4Acc/tests/ShallowWater2D/RefactoredSources/SConstruct.rf4a", line 12:
    envC=Environment(CC=CC,CPPPATH=[]);
  File "/usr/local/lib/scons-3.0.0/SCons/Environment.py", line 982:
    apply_tools(self, tools, toolpath)
  File "/usr/local/lib/scons-3.0.0/SCons/Environment.py", line 107:
    env.Tool(tool)
  File "/usr/local/lib/scons-3.0.0/SCons/Environment.py", line 1789:
    tool(self)
  File "/usr/local/lib/scons-3.0.0/SCons/Tool/__init__.py", line 292:
    self.generate(env, *args, **kw)
  File "/usr/local/lib/scons-3.0.0/SCons/Tool/default.py", line 40:
    for t in SCons.Tool.tool_list(env['PLATFORM'], env):
  File "/usr/local/lib/scons-3.0.0/SCons/Tool/__init__.py", line 1186:
    c_compiler = FindTool(c_compilers, env) or c_compilers[0]
  File "/usr/local/lib/scons-3.0.0/SCons/Tool/__init__.py", line 1091:
    if t.exists(env):
  File "/usr/local/lib/scons-3.0.0/SCons/Tool/gcc.py", line 64:
    return detect_version(env, env.Detect(env.get('CC', compilers)))
  File "/usr/local/lib/scons-3.0.0/SCons/Environment.py", line 1492:
    path = self.WhereIs(prog)
  File "/usr/local/lib/scons-3.0.0/SCons/Environment.py", line 1809:
    path = SCons.Util.WhereIs(prog[0], path, pathext, reject)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/UserList.py", line 31:
    def __getitem__(self, i): return self.data[i]

I'm not familiar with python so if there's something with my python setup, I can also try this on a different system. In case it helps, I'm on macOS. I can try it on Linux if that's better for any reason.

Below are three additional comments that do not have to be addressed for for me to recommend acceptance to JOSS, but might be helpful feedback if you're interested.

  1. Besides the Fortran 90 declarations, this example uses other non-F77 features , including
    • end do do-loop termination (although F77 allowed end do for while and do while loops)
    • module and use are F90
    • the == in a logical expression
    • a C-preprocessor macro
    • free format source

The F77 standard is available here. I don't see this as a problem. I'm just clarifying.

  1. It would be nice to have an example that doesn't require an external build package unless that build package is something that will always be needed to use RefactorF4Acc, which is not the case. Fortunately SCons was very easy for me to install, but it appears that the code compiles fine without it so I suggest that the README.md recommend users to simply execute something like the following if they don't want to install scons:

    export FFLAGS="-cpp -Ofast -m64 -Wall -ffree-form -fconvert=big-endian -mcmodel=medium"
    gfortran $FFLAGS param.f95 sub.f95 main.f95 -o shallow_water_2D

    I think that will lead to the same result without requiring that users download and install another package.

  2. The 2D Shallow Water example has 335 lines:

    $ wc -l *.f95
     142 main.f95
      22 param.f95
     171 sub.f95
     335 total

    Maybe you're weren't counting comments when you wrote "180 lines?" Either way, it's fine, but I do have several suggestions for shortening it. Although I see the value in using a real-world application code as the short example, what I'm suggesting is really more for didactic purposes. You could certainly base it on this same code, but strip out lots of unnecessary aspects, including but not limited to the following:

    • the commented 9-line IF block that starts at line 119 in the main program
    • the commented 6-line DO loop at line 35 in the sub module
    • the C-preprocessor macro at line 135 in sub.f95 if it's unnecessary, which I think is the case.
    • the 7 unused variables identified in gfortran warning messages: hmax,time,dtmax,n,ntot,nout, dummy

There's more that I would do to simplify it. If you like, I would be glad to submit a pull request with a version of the shallow water code that I would recommend using. If possible, my goal would be to get it to fit comfortably on one or two screens (hopefully fewer than 200 lines). It would also be nice if the corresponding README.md gave a brief description of the specific changes between the original code and the code that RefactorF4Acc produces. I would also be glad to help with writing that description and then I would suggest linking to it in a top-level GETTING_STARTED.md. Let me know if any of this is of interest.

wimvanderbauwhede commented 5 years ago

Thanks for the comments. (1) You are right of course, I was simplifying a bit when I said the code only used F95 type declarations. I am familiar with the F77 standard :-) I have included the differences you list in the README.md

(2) The SCons error is because you didn't have $CC defined. You don't need it so I removed it. I also made a script "generate.sh" which does not build the code, so it does not need SCons. And I added the manual build line to the README.md

(3) I use cloc to count lines of code, and it gives

[wim@HackBookPro fortran]$ cloc --include-lang='Fortran 95' .
       7 text files.
       7 unique files.                              
       4 files ignored.

github.com/AlDanial/cloc v 1.72  T=0.03 s (110.6 files/s, 12354.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Fortran 95                       3             74             81            180
-------------------------------------------------------------------------------
SUM:                             3             74             81            180
-------------------------------------------------------------------------------

I would appreciate if you would submit a pull request for a shorter version of the code. I did the obvious things and it is now

[wim@HackBookPro fortran]$ wc -l *.f95
     119 main.f95
      19 param.f95
     149 sub.f95
     287 total

I'm not sure what the difference would be between GETTING_STARTED.md and README.md, and also not really about what changes you'd like to be described in the README.md, could you explain?

Thanks!

rouson commented 5 years ago

The changes look great! I believe the code changes suffice so there's no need for me to submit a pull request with simplified code. I'll close this issue now and will submit the other related pull requests with a GETTING_STARTED.md and a few other suggested documentation changes.