Skip to content

Conversation

@dhaumont
Copy link
Contributor

@dhaumont dhaumont commented Oct 16, 2025

This PR is a refactoring to eliminate duplicated code between trltog and trgtol in the cpu branch.

The duplicated codehas been extracted into new routines in a new module trgl_mod, and they are called from trltog and trgtol.

@dhaumont
Copy link
Contributor Author

@samhatfield All your suggestions have been pushed. Looking forward to your PR into my branch

@samhatfield
Copy link
Collaborator

Unfortunately it wasn't possible to collapse TRGTOL and TRGTOL_COMM into one subroutine, or at least I couldn't find a way (same issue with TRLTOG). The issue is that the stack versions of ZCOMBUFS/R have a shape which depends on YDBUFS%INSEND, and this must first be defined in TRGTOL. So, you have an array whose dimensions must be known at compile time, but the dimensions are only defined at run time...

@samhatfield
Copy link
Collaborator

Overall I think this is a nice change and I like how the communication routines are much shorter and the complicated index calculations are done elsewhere. Now TRGTOL/TRLTOG are mostly about communication which makes them easier to read IMO. I'd be happy to merge this, but I'd like to hear an opinion of @RyadElKhatibMF first. I also think we should take this chance to comment the code a bit more.

@RyadElKhatibMF
Copy link
Contributor

yes, it looks fine.

Fix the bug that the size calculation for reallocation was incorrect due
to the Z_HEAP buffer allocated with a lower bound of -1
see ecmwf-ifs#317 (comment)
@samhatfield
Copy link
Collaborator

I'll make one last check and then I think this is ready to merge.

@samhatfield samhatfield added enhancement New feature or request tidying Tidying up code or removing unused features labels Jan 13, 2026
@samhatfield samhatfield added this to the 1.8.0 milestone Jan 13, 2026
@samhatfield
Copy link
Collaborator

samhatfield commented Jan 13, 2026

I realised after I started testing that there is still a bug in the checksum dumps for spectral fields. I'd like to test trltog_clean properly looking at checksums etc., but this isn't possible until the bug is fixed, so I opened a new PR here. Let's review and merge that first, then we can merge develop into trltog_clean so we can properly test it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor enhancement New feature or request tidying Tidying up code or removing unused features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants