Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory layout in gas optics kernels #161

Closed
RobertPincus opened this issue Mar 30, 2022 · 2 comments
Closed

Memory layout in gas optics kernels #161

RobertPincus opened this issue Mar 30, 2022 · 2 comments

Comments

@RobertPincus
Copy link
Member

An issue received via email:

From: Thomas Jahns <[jahns@dkrz.de](mailto:jahns@dkrz.de)>
Sent: Monday, March 7, 2022 9:57 AM
To: [rrtmgp@aer.com](mailto:rrtmgp@aer.com) <[rrtmgp@aer.com](mailto:rrtmgp@aer.com)>
Cc: Claudia Frauen <[frauen@dkrz.de](mailto:frauen@dkrz.de)>; Jörg Behrens <[behrens@dkrz.de](mailto:behrens@dkrz.de)>
Subject: Memory layout in rrtmgp/kernels/mo_gas_optics_kernels.F90
 
Hi,

I was roped in by some colleagues to look at some issues in the mo_gas_optics_kernels module and noticed two other, independent problems for compiler optimization of the module:

1. all routines are made public and bind(c) which prevents inline for compilers adhering to strict semantics for symbols that can be interpositioned. Would it be possible to change the declaration of interpolate3D_byflav to private and remove the bind(c) to permit Fortran assumed shape arguments.

2. the above would allow to change the fields from e.g.

integer, dimension(2, ncol,nlay,nflav), intent(out) :: jeta
real(wp), dimension(2, ncol,nlay,nflav), intent(out) :: col_mix
real(wp), dimension(2,2,2,ncol,nlay,nflav), intent(out) :: fmajor
real(wp), dimension(2,2, ncol,nlay,nflav), intent(out) :: fminor

to

integer, dimension(ncol,2, nlay,nflav), intent(out) :: jeta
real(wp), dimension(ncol,2, nlay,nflav), intent(out) :: col_mix
real(wp), dimension(ncol,2,2,2,nlay,nflav), intent(out) :: fmajor
real(wp), dimension(ncol,2,2, nlay,nflav), intent(out) :: fminor

and enable the compiler to process the data with multiple packed SIMD-streams instead of emitting instructions for vector gather/scatter operations of data that is essentially in AoS layout.

I think I could submit a corresponding code change if the idea seems possible in general.

Kind regards, Thomas
@RobertPincus
Copy link
Member Author

An initial response by email:

 It should be possible to remove C bindings from any routines that aren’t called by the Fortran front-ends. I’m skeptical, though, that using assumed-shape will lead to better performance than using assumed-size.

@RobertPincus RobertPincus mentioned this issue Apr 9, 2022
Merged
RobertPincus added a commit that referenced this issue Apr 9, 2022
@RobertPincus
Restrict visibility, end type labels; fix Python in CI (#165)
d5aad35 
1. Narrow the range of public procedures in the Fortran kernel modules; the remaining ~12 are the kernel API.
2. Remove C bindings from any non-public functions (#161)
3. C-bindings are now prefixed with "rte_" or "rrtmgp_" (#158)
4. Add labels to end type statements (#164)
5. Remove unneeded array-reordering kernels
6. Update use of conda in continuous integration using Github Actions
@RobertPincus
Copy link
Member Author

Given the lack of interest we'll close this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@RobertPincus