Make data device resident in GPNORM_TRANS

[awnawab]

This PR makes data device resident throughout the execution of GPNORM_TRANS, including the addition of gpu aware comms.

[Copilot]

Pull request overview

This PR extends GPNORM_TRANS to better support GPU execution by keeping key working data resident on the device across the routine, and by enabling GPU-aware point-to-point communications (with optional raw MPI support). It also updates the public interface so callers can indicate when PGP is already device-resident.

Changes:

• Added optional LPGP_ON_GPU argument to GPNORM_TRANS (interface + CPU/GPU implementations).
• Refactored the GPU implementation to maintain device-resident temporaries across the workflow and integrate GPU-aware send/recv paths.
• Kept CPU implementation interface-compatible while leaving behavior unchanged.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/trans/include/ectrans/gpnorm_trans.h	Extends the public interface to include optional LPGP_ON_GPU.
src/trans/gpu/external/gpnorm_trans.F90	Major GPU-path refactor: persistent device data usage + GPU-aware comms + optional raw MPI handling.
src/trans/cpu/external/gpnorm_trans.F90	Adds the optional argument for interface compatibility (unused on CPU).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[samhatfield]

I suggest we broaden the scope slightly and call it "tidy of GPNORM_TRANS" on GPU, and also delete gpnorm_trans_gpu.F90. The latter has never been functional and will be redundant once this is merged.

[samhatfield]

I added a test suite for GPNORM_TRANS which checks whether the average computed matches the (0, 0) mode of a corresponding spectral array. Currently it only tests LPGP_ON_GPU=.FALSE..

[samhatfield]

Failure on LUMI. Is this directive correct @awnawab?

gpnorm_trans.F90:224-228

#ifdef OMPGPU
!$OMP TARGET DATA MAP(PRESENT,ALLOC:ZAVE,ZMINGL,ZMAXGL,ZMINGPN,ZMAXGPN,ZAVEG,ZMING,ZMAXG) &
!$OMP& MAP(TO:IVSETS,IVSETG) &
!$OMP& MAP(PRESENT,ALLOC:PREEL_REAL,F_RW,D_NSTAGTF,D_NPTRLS,G_NLOEN)
#endif

Arrays ZAVE etc. -> those haven't yet been allocated on device right? Should they be MAP(TO:...)?

[awnawab]

Failure on LUMI. Is this directive correct @awnawab?

gpnorm_trans.F90:224-228

#ifdef OMPGPU
!$OMP TARGET DATA MAP(PRESENT,ALLOC:ZAVE,ZMINGL,ZMAXGL,ZMINGPN,ZMAXGPN,ZAVEG,ZMING,ZMAXG) &
!$OMP& MAP(TO:IVSETS,IVSETG) &
!$OMP& MAP(PRESENT,ALLOC:PREEL_REAL,F_RW,D_NSTAGTF,D_NPTRLS,G_NLOEN)
#endif

Arrays ZAVE etc. -> those haven't yet been allocated on device right? Should they be MAP(TO:...)?

I suspect it's the present map modifier. Looking back at it, the only function that serves is a safety check to abort if that symbol isn't already present, and of course it should throw an error for variables yet to be allocated on device. I think removing it is the right fix. Thanks a lot for adding the test! Otherwise this would have gone uncaught.

[samhatfield]

We have now an error on LUMI only affecting mpi2 tests, which is not at all obvious. I'll have to reproduce it and interactively debug it on LUMI.

[awnawab]

Considering the actual use case for this is in ifs coupled builds, and there's no pressing need to do that on lumi, we can exclude these 4 tests on lumi, leave an issue and then I'm happy to try to debug it in a non time pressing manner.

[samhatfield]

Looks like you spotted a bug @awnawab ...? Tests passing on LUMI now.

[awnawab]

Yeah, I'd misunderstood the difference between use_device_ptr and use_device_addr, all good now.

[samhatfield]

Then I'll take a proper look at gpu/external/gpnorm_trans.F90 and give a final review.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[samhatfield]

Does this really matter @awnawab ?

[awnawab]

No I wouldn't think so. In the forecast timestep hotpath, running those consistency will incur an overhead, and it feels like overkill. However outside the hotpath, where we also call GPNORM_TRANS, the data in those cases is on host anyway, and there's little overhead for those checks.