stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 28 forks source link

Not all examples can compile #1474

Open LonelyCat124 opened 3 years ago

LonelyCat124 commented 3 years ago

While trying to enable my OpenMP taskloop/wait code to both generate and compile, I discovered that some of the examples code does not compile with only the dl_esm_inf dependency.

The one I tested was eg1 in gocean, the changes I fixed up in my branch:

  1. wp in various .f90 files was changed to go_wp
  2. type(arg) in compute_cu.f90 was changed to type(go_arg)

The main thing I couldn't fix to work correctly was shallow_alg.f90. Some of the same changes were required, however there were a few things that were definitely invalid:

  1. Some modules that were used don't exist in dl_esm_inf, and thus nor do some functions
  2. model_grid(..) should use GO_ arguments
  3. itmax, dt and tdt are undefined.

I fixed 2 & 3 where I could.

I've haven't explored other examples, but they may also not be compile-able.

hiker commented 3 years ago

Gee, that really surprised me, I thought we did run compile tests regularly. Having a quick look shows that gocean/eg1 has no support for compilation:

make compile
...
No compilation targets for example gocean/eg1

So I think this is at least a known issue 😈 I don't know why we don't (at least) compile that example, and I am all in favour to fixing it.

You mentioned that you fixed some of the issues, so I assume it doesn't make any sense for me to start working on it. But feel free to create a branch with your fixes so far and assign it to me, I can have a look at the rest.

arporter commented 3 years ago

Thanks @hiker. Once #1415 is merged (it's doing its final CI run now), these changes that Aidan has made will be on master.

TeranIvy commented 3 years ago

One other thing that I noticed is that make clean in examples does not remove two files in gocean/eg1 directory:


gocean/eg1/ompt_alg.f90
gocean/eg1/ompt_psy.f90
LonelyCat124 commented 3 years ago

Not being able to clean is my bad - I have no idea how this functionality works though so I can't fix it :(

hiker commented 2 years ago

I am working on this now. I will comment out the calls for which we have no source code (but leave them as comments in case someone feels like completing the examples).

@arporter - some of the kernels include manual 'invoke' subroutines, e.g.:

  !> Manual implementation of the code needed to invoke
  !! compute_cu_code().
  subroutine invoke_compute_cu(cufld, pfld, ufld)
  ...
    ! Note that we do not loop over the full extent of the field.
...
    !   vi-1j+1--fij+1---vij+1---fi+1j+1
    !   |        |       |       |
    !   |        |       |       |
    !   Ti-1j----uij-----Tij-----ui+1j
    !   |        |       |       |
    !   |        |       |       |
    !   vi-1j----fij-----vij-----fi+1j
    !   |        |       |       |
    !   |        |       |       |
    !   Ti-1j-1--uij-1---Tij-1---ui+1j-1
    !

    do J=cufld%internal%ystart, cufld%internal%ystop
       do I=cufld%internal%xstart, cufld%internal%xstop

          call compute_cu_code(i, j, cufld%data, pfld%data, ufld%data)
       end do
    end do

  end subroutine invoke_compute_cu

It appears safe for me to remove the implementation. Do you want me to keep the comments, or can I remove them as well?

arporter commented 2 years ago

Seems a shame but we don't need them now :-) They probably exist in PSycloneBench anyway.

arporter commented 1 year ago

I think this issue can be closed now. Do you agree @hiker?