ecmwf-ifs / ectrans

Global spherical harmonics transforms library underpinning the IFS
Apache License 2.0
15 stars 30 forks source link

Implement SIZEOF for NAG. #78

Closed DJDavies2 closed 2 months ago

DJDavies2 commented 3 months ago

For https://github.com/ecmwf-ifs/ectrans/issues/77. This implements the same macro as for NEC.

samhatfield commented 3 months ago

Looks good to me.

samhatfield commented 3 months ago

I think instead of adding a SIZEOF implementation we should just refactor the code so we don't need it at all. Consider one of the two places where we used SIZEOF:

IF( .NOT.ALLOCATED(ZCLONEA(IMLOC)%COMMSBUF) )THEN
    ICLONELEN=IKOUNT(JSETV)
    ALLOCATE(ZCLONEA(IMLOC)%COMMSBUF(ICLONELEN))
    ZCLONEA(IMLOC)%COMMSBUF(1:ICLONELEN) = ZRCVBUTFV(1:ICLONELEN,JSETV)
    CALL UNPACK_BUTTERFLY_STRUCT(S%FA(IMLOC)%YBUT_STRUCT_A,ZCLONEA(IMLOC))
ENDIF
IF(ALLOCATED(ZCLONEA(IMLOC)%COMMSBUF) ) THEN
    IF( SIZEOF(ZCLONEA(IMLOC)%COMMSBUF) > 0 ) DEALLOCATE(ZCLONEA(IMLOC)%COMMSBUF)
ENDIF

Let me rewrite this in pseudocode:

 IF (.NOT. ALLOCATED(ARRAY)) THEN
     ICLONELEN = IKOUNT(JSETV)
     ALLOCATE(ARRAY(ICLONELEN))
    ! Some stuff 
ENDIF
! ARRAY must be allocated so why bother having this IF?
IF (ALLOCATED(ARRAY)) THEN
    ! Even so, we don't need SIZEOF because we can just check whether ICLONELEN is > 0
    IF( SIZEOF(ZCLONEA(IMLOC)%COMMSBUF) > 0 ) DEALLOCATE(ZCLONEA(IMLOC)%COMMSBUF)
ENDIF

I will have a go at this refactor and submit a new PR.

wdeconinck commented 2 months ago

Alternative PR #89 was merged that solves this issue differently