Remove dependence on the global modules from the cloudsc-fortran #35
Conversation
piotrows
changed the title
There was a problem hiding this comment.
Overall looks good, and gets us around a very big headache problem, and is in line with developments brought to us from Meteo-France.
I've marked to small cosmetic changes, one about the location of the additional loader code (not in the dwarf head program), and the additional integer flags that could also be wrapped, rather than extending the subroutine signature. After that, this should be GTG.
| TYPE(TECLDP) ,INTENT(INOUT) :: YDECLDP | ||
| TYPE(TEPHLI) ,INTENT(INOUT) :: YDEPHLI | ||
|
|
||
| YDECLDP%RKCONV = YRECLDP%RKCONV |
There was a problem hiding this comment.
Should these loader routines not live in the respective modules, like YOETHF, YOEMCST, etc.? We already have global loader functions in there, and we could probably add type-specific ones there (either YDETHF%INITIALIZE() or add something like YOETHF_LOAD_PARAMETERS(YDETHF) to the existing loader routine? This would avoid polluting the high-level entry routines with this.
| ! Call the driver to perform the parallel loop over our kernel | ||
| CALL CLOUDSC_DRIVER(NUMOMP, NPROMA, GLOBAL_STATE%KLEV, NGPTOT, NGPTOTG, & | ||
| CALL CLOUDSC_DRIVER( NUMOMP, NPROMA, GLOBAL_STATE%KLEV, NGPTOT, NGPTOTG, & | ||
| & NCLDQV, NCLDQL, NCLDQR, NCLDQI, NCLDQS, NCLV, & |
There was a problem hiding this comment.
The global cloud vector indexes/sizes can probably also be wrapped in a derived type to avoid passing all of them down.
There was a problem hiding this comment.
@mlange05 Is derived type necessary or it could be just a vector ? Perhaps it could be hardcoded (e.g. called integer :: TNCLV(NCLV)) in MODULE YOECLDP ?
There was a problem hiding this comment.
These are named integer indexes. Why would we want to create yet another pattern, instead of using the same as for the other module-level parameters? Passing them as a derived type object we can use associates in the subroutine body, following the existing convection, no?
There was a problem hiding this comment.
@mlange I understand that explicit declaration of NCLDQV, NCLDQL, NCLDQR, NCLDQI, NCLDQS, NCLV must stay as they are in the yoecldp, without putting them in the derived type envelope. So the information needs to be duplicated somewhere, what about putting them as members of TECLDP, so no extra type definition is need ?
There was a problem hiding this comment.
Yes, adding them to TECLDP and setting them in the initialisation routine should work nicely I think.
…tialization of parameters in parameter containers (i.e. yrthf, yrecldp, yrcst) to the respective derived type modules.
|
[DO NOT MERGE UNTIL RESOLVED!] Not quite sure what's going on here, but this change seems to drop CPU performance of the changed variant significantly; I'm getting a drop from around 88GF/s to 58GF/s (Intel, single socket)! Please hold off on review/merge until this is understood. |
|
This may due to the fact that cloud indexes are now unknown at the compile time, investigation in progress. Also I have converted it to draft, I think I extra minor change about passing on NCLV to driver is needed for the python driver. |
…CATED...) checks to the parameter derived type loaders, remove unused YREPHLI
|
I have checked that the performance on Mac+gfortran 12 doesn't depend significantly on the availability of cloud indices 1...5 at compile time. To be checked on ATOS. |
|
I have checked that with Intel the current commit has matching performance to the 'Apple' commit. It seems that with gcc the performance is less dependent on the strategy of passing of indices. |
This commit removes the dependence on the parameters from global modules from the clouds computation.