64 simple mpi support

[hiker]

This PR adds very simple (and not particularly efficient) MPI support:

1. The constructor of fields takes an optional argument which is the global array (on each PE). Each PE then copies the data from its own subdomain into the field. That requires that the global array is created on each PE (instead of doing it on one PE, and then broadcasting the data), but it works really well with my example for the PSyclone training. The advantage of providing the global array on each PE is that there is no need during input handling to test if (on_master()), it is simply:

    allocate(initial_state(n_cols, n_rows))

    call get_initial_state(initial_state, n_rows, n_cols)

    initial = r2d_field(grid, grid_points = GO_T_POINTS,   &
                        init_global_data = initial_state)

2. There is a new function to 'gather' a distributed array back into a 2d field. The output function still needs a if(on_master()), (so that only one process outputs the results)

[hiker]

Maybe an example how output now works:

        call field%gather_inner_data(global_data)

        if (on_master()) then
            do j=lbound(global_data, 2), ubound(global_data, 2)
                write(*,"(99F2.0)") global_data(1:ubound(global_data,1), j)
            enddo
            write(*,*)
        endif
        deallocate(global_data)

[hiker]

Ready for a first review. I am happy to discuss if this approach should be too limited (admittedly it was tweaked to simplify the PSyclone training).

[hiker]

@arporter Hi Andy, would it be possible to get this reviewed in the near future? I am working on the PSyclone training, and atm I have to point to an external dl_esm_info repo (instead of the internal PSyclone one), and have to take care when compiling any psydata lib to also point to the same external repo (otherwise I get linking problems due to the additional functions).
And esp., if you don't like this approach, I need to redo my hands-on session.

[arporter]

Apologies for forgetting about this Joerg. Generally I think it looks good although I have to admit it feels like a bit of a retrograde step to allow global arrays (I think we used to have them and then I removed them). However, since you've made them optional then I think it's OK as long as we're clear that the library can be used without them. (This is important if we want a simple benchmark capable of running with a very large domain.)
Please could you add a test of the new reduction functionality under finite_difference/tests/dist_mem. Apart from that, there's only a couple of minor things.

[hiker]

Fixed, but am happy to discuss the requirement of the tmask in the telco tonight.

[hiker]

I grep'ed for t_mask in the source files, and there seems to be no other use of the tmask, except for copying it in the grid constructor (it is passed as a parameter somewhere, but then not actually used). So I don't see a good reason to insist on having a tmask, since all of the code should be working just fine if there is no tmask.

[arporter]

Thanks for adding the tests - they're nice :-)
After much thought and examination of PSycloneBench, I've concluded that we do need the T-mask so have requested you revert some of those changes.
If you agree then once that's done this is ready to go.

[hiker]

finite_difference/src/grid_mod.f90
call gocean_stop('grid_init: ERROR: No T-mask supplied and '// &
'grid does not have periodic boundary conditions!')
end if
!> TODO add support for PBCs in parallel
Member
@arporter arporter 3 days ago
I'd still like to keep the consistency check - an error message is always helpful :-)

Sorry, I don't know how to quote-reply to that comment, I don't have the usual 'reply' option.

As far as I can see, the state if we don't have a T-mask is either:

1. We have periodic boundary conditions. Then indeed you don't need to supply a tmask ... but MPI execution aborts later(!) at a halo exchange since this is not yet supported.
2. We don't have periodic boundary conditions. The above test will then abort the execution as well.

Given that 2. actually works, shouldn't it be the other way round? We abort early if we detect an unsupported configuration (instead of running and aborting later)?

I'll try to have a chat with you about this tonight. I feel we might have a misunderstanding. My change only makes the T-mask truly optional, all existing code that uses a T-mask will continue to work, but in addition we can also use it without T-mask, which we can't atm. The only configuration that would be a problem is if an application does not provide a tmask, but in a kernel then asks for it (though we could detect this and then either provide with an error, or even provide a dummy tmask, i.e. same size as the grid, filled with 1's)

[hiker]

Hmm - I found a comment 'Please update the date.' for grid_mod.f90 ... since it had no license, I've added the usual license to the top of the file. But when trying to comment on that in the review here, it turns out that comment was for field_mod (which I had already fixed). Given that it seems correct to add a proper license for grid_mod, I left it in.

[hiker]

I think I have updated everything as discussed. Back to you :)

[arporter]

All good now - thanks.
I've checked the make_gen target of PSycloneBench with this branch and it was fine.
The tests added in this PR also work fine.
Will proceed to merge.