clawpack / classic

Classic single-grid Fortran routines
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
11 stars 25 forks source link

Add CMake buildsystem, fix sources to compile #3

Closed certik closed 11 years ago

certik commented 11 years ago

The provided Makefile does not work for me:

ondrej@eagle:~/repos/classic/tests/advection(cmake)$ make
Makefile:57: /src/Makefile.common: No such file or directory
make: *** No rule to make target `/src/Makefile.common'.  Stop.

so this PR implements a CMake build system. Now it builds, thought here are lots of warnings and errors. This PR fixes all errors, so that sources compile. The warnings are:

ondrej@eagle:~/repos/classic(cmake)$ make
Scanning dependencies of target classic
[  6%] Building Fortran object src/CMakeFiles/classic.dir/utils.f90.o
[ 12%] Building Fortran object src/CMakeFiles/classic.dir/clawdata_module.f90.o
[ 18%] Building Fortran object src/CMakeFiles/classic.dir/precision_module.f90.o
[ 25%] Building Fortran object src/CMakeFiles/classic.dir/1d/solution_module.f90.o
[ 31%] Building Fortran object src/CMakeFiles/classic.dir/1d/geometry_module.f90.o
[ 37%] Building Fortran object src/CMakeFiles/classic.dir/1d/solver_module.f90.o
/home/ondrej/repos/classic/src/1d/solver_module.f90:251.57:

            call set_boundary_conditions(solution,solver)
                                                         1
Warning: Procedure 'set_boundary_conditions' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:254.45:

            call before_step(solution,solver)
                                             1
Warning: Procedure 'before_step' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:258.75:

                call source_term(t_old, solver%dt / 2.d0, solution, solver)
                                                                           1
Warning: Procedure 'source_term' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:262.49:

            call hyperbolic_step(solution,solver)
                                                 1
Warning: Procedure 'hyperbolic_step' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:267.82:

         call source_term(solution%t + solver%dt,solver%dt,solution,solver)
                                                                           1       
Warning: Procedure 'source_term' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:269.98:

urce_term(solution%t + solver%dt * 0.5d0,solver%dt * 0.5d0,solution,solver)
                                                                           1                       
Warning: Procedure 'source_term' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/solver_module.f90:156.24:

        self%limiters = clawdata%limiters
                        1
Warning: Possible change of value in conversion from REAL(8) to INTEGER(4) at (1)
[ 43%] Building Fortran object src/CMakeFiles/classic.dir/1d/rp1_interface.f90.o
[ 50%] Building Fortran object src/CMakeFiles/classic.dir/1d/before_step.f90.o
[ 56%] Building Fortran object src/CMakeFiles/classic.dir/1d/hyperbolic_step.f90.o
/home/ondrej/repos/classic/src/1d/hyperbolic_step.f90:108.45:

                             solver%limiters)
                                             1
Warning: Procedure 'limiter' called with an implicit interface at (1)
/home/ondrej/repos/classic/src/1d/hyperbolic_step.f90:115.16:

                f(m,i) = f(m,i) + 0.5d0 * abs(s(mw,i))      &
                1
Warning: The FORALL with index 'mw' is not used on the left side of the assignment at (1) and so might cause multiple assignment to this object
[ 62%] Building Fortran object src/CMakeFiles/classic.dir/1d/set_aux.f90.o
[ 68%] Building Fortran object src/CMakeFiles/classic.dir/1d/boundary_conditions.f90.o
[ 75%] Building Fortran object src/CMakeFiles/classic.dir/1d/limiter.f90.o
[ 81%] Building Fortran object src/CMakeFiles/classic.dir/1d/qinit.f90.o
[ 87%] Building Fortran object src/CMakeFiles/classic.dir/1d/source_term.f90.o
Linking Fortran static library libclassic.a
[ 87%] Built target classic
Scanning dependencies of target main
[ 93%] Building Fortran object tests/advection/CMakeFiles/main.dir/rp1_advection.f90.o
[100%] Building Fortran object tests/advection/CMakeFiles/main.dir/main.f90.o
Linking Fortran executable main
[100%] Built target main

Some of them seem to reveal real problems in the code, for example the self%limiters = clawdata%limiters one. The fact that the code uses implicit interface is very bad, because the compiler can't check the types. This should not be used in modern Fortran. The solution is to at least provide an interface block and import this block, or (better) move all global subroutines into modules and import the modules.

When I try to run the test:

ondrej@eagle:~/repos/classic/tests/advection(cmake)$ ./main 
*** in opendatafile, file not found:./setprob.data

It fails due to missing input file.

Another problem is that the main.f90 file is referencing controller_module.f90 which is not in the repository, so i had to comment it out, together with run_simulation() which I assume is declared in it.

@mandli, could you please review this?

mandli commented 11 years ago

I agree on explicit interfaces, just had not gotten a chance to stick them somewhere it makes sense. One issue is that a user needs to override some of these functions (before_step for instance) and sticking this into a module is janky. Really need to just have an interface defined for all possibly user overridden code and have function pointers available. This is perhaps not ideal though as f2py does not do interface blocks.

The CMake build system is not needed as the Makefile.common referenced is in the CLAWUtil repository.

certik commented 11 years ago

Ok. I am closing this PR, as all the fixes are in and I'll open a new PRs with any other improvements.