Skip to content

Align GEOS_IrradGridComp.F90 with the MAPL3 port's ACG usage and structure - #98

Open
pchakraborty wants to merge 2 commits into
developfrom
refactor/pchakrab/irrad-mapl2-acg-and-structure
Open

Align GEOS_IrradGridComp.F90 with the MAPL3 port's ACG usage and structure#98
pchakraborty wants to merge 2 commits into
developfrom
refactor/pchakrab/irrad-mapl2-acg-and-structure

Conversation

@pchakraborty

Copy link
Copy Markdown

Brings the MAPL2 baseline closer to the MAPL3 port branch so the two stay easier to diff and reason about together, without changing any numerics.

  • ACG-fy SetServices: replace hand-written MAPL_AddImportSpec/ AddExportSpec/AddInternalSpec calls with ACG-generated #include "Irrad_{Import,Export,Internal}___.h", driven by a new Irrad_StateSpecs.rc and the mapl_acg() CMake hook (matching the MAPL3 port's SetServices). Manual AERO import and the dynamic RATS_DIAGNOSTICS export block are left hand-written, as in MAPL3.

  • Extract compute_provider_aerosol_optics as a standalone module-level routine from LW_Driver's RADIATIVELY_ACTIVE_AEROSOLS block, mirroring the MAPL3 port's extraction of the same block (ESMF_AttributeGet in place of MAPL3's ESMF_Info-based API).

  • Extract compute_lw_aer_optics, compute_lw_cloud_optics_mcica, compute_lw_gas_optics, compute_lw_rte, and PROCESS_RRTMGP_LW_BLOCK out of Run's nested scope into standalone module-level routines, matching MAPL3's module layout. All five already took every crossing variable as an explicit argument, so this is a pure relocation with no signature or logic changes.

  • Move scheme-specific LW_Driver setup closer to where it's used, mirroring the MAPL3 port's LW_Driver restructuring: CHOU-only emissivity/vegetation hardwiring now lives inside the USE_CHOU branch, and RRTMG-only RATS-toggle parsing plus the CLDTTLW/CLDHILW/ CLDMDLW/CLDLOLW export pointers now live inside the USE_RRTMG branch. Shared setup (PL, FCLD, CO2_FIXED, LCLDMH/LCLDLM) stays unconditional since CHOU/RRTMG(/P) all need it.

  • Move aerosol-only setup (RH, the IRRAD aerosol-band OFFSET, and TAUA/SSAA/ASYA allocation+zeroing) inside the ---AEROSOLS timer block, alongside the AERO provider call. PL stays outside since RRTMG/RRTMGP also need it.

  • Collapse RC=STATUS / VERIFY_(STATUS) pairs into the single-line _RC macro throughout (142 call sites), matching MAPL3's error- handling style. RC=STATUS/VERIFY_(STATUS) pairs deliberately left unconverted (RATS diagnostics' NotFoundOK-style unverified MAPL_GetPointer calls, multi-branch calls sharing one VERIFY_ after an if/else chain) are untouched.

  • Drop purely decorative dash-only and equals-only comment-divider lines throughout (BOP/EOP/BOS/EOS protex markers untouched).

No behavioral changes intended; this is a structural/style-alignment pass ahead of further MAPL2/MAPL3 comparison work.

…le structure

Brings the MAPL2 baseline closer to the MAPL3 port branch so the two
stay easier to diff and reason about together, without changing any
numerics.

- ACG-fy SetServices: replace hand-written MAPL_AddImportSpec/
  AddExportSpec/AddInternalSpec calls with ACG-generated
  #include "Irrad_{Import,Export,Internal}___.h", driven by a new
  Irrad_StateSpecs.rc and the mapl_acg() CMake hook (matching the
  MAPL3 port's SetServices). Manual AERO import and the dynamic
  RATS_DIAGNOSTICS export block are left hand-written, as in MAPL3.

- Extract compute_provider_aerosol_optics as a standalone module-level
  routine from LW_Driver's RADIATIVELY_ACTIVE_AEROSOLS block, mirroring
  the MAPL3 port's extraction of the same block (ESMF_AttributeGet/
  ESMF_MethodExecute in place of MAPL3's ESMF_Info-based API).

- Extract compute_lw_aer_optics, compute_lw_cloud_optics_mcica,
  compute_lw_gas_optics, compute_lw_rte, and PROCESS_RRTMGP_LW_BLOCK
  out of Run's nested scope into standalone module-level routines,
  matching MAPL3's module layout. All five already took every crossing
  variable as an explicit argument, so this is a pure relocation with
  no signature or logic changes.

- Move scheme-specific LW_Driver setup closer to where it's used,
  mirroring the MAPL3 port's LW_Driver restructuring: CHOU-only
  emissivity/vegetation hardwiring now lives inside the USE_CHOU
  branch, and RRTMG-only RATS-toggle parsing plus the CLDTTLW/CLDHILW/
  CLDMDLW/CLDLOLW export pointers now live inside the USE_RRTMG
  branch. Shared setup (PL, FCLD, CO2_FIXED, LCLDMH/LCLDLM) stays
  unconditional since CHOU/RRTMG(/P) all need it.

- Move aerosol-only setup (RH, the IRRAD aerosol-band OFFSET, and
  TAUA/SSAA/ASYA allocation+zeroing) inside the ---AEROSOLS timer
  block, alongside the AERO provider call. PL stays outside since
  RRTMG/RRTMGP also need it.

- Collapse RC=STATUS / VERIFY_(STATUS) pairs into the single-line
  _RC macro throughout (142 call sites), matching MAPL3's error-
  handling style. RC=STATUS/VERIFY_(STATUS) pairs deliberately left
  unconverted (RATS diagnostics' NotFoundOK-style unverified
  MAPL_GetPointer calls, multi-branch calls sharing one VERIFY_ after
  an if/else chain) are untouched.

- Drop purely decorative dash-only and equals-only comment-divider
  lines throughout (BOP/EOP/BOS/EOS protex markers untouched).

No behavioral changes intended; this is a structural/style-alignment
pass ahead of further MAPL2/MAPL3 comparison work.
@pchakraborty pchakraborty self-assigned this Aug 14, 2026
@pchakraborty
pchakraborty requested review from a team as code owners August 14, 2026 11:53
@pchakraborty pchakraborty added the 0 diff structural Structural changes to repository that are zero-diff label Aug 14, 2026
Comment on lines +1045 to +1064
call MAPL_GetPointer(IMPORT, PLE, 'PLE', _RC)
call MAPL_GetPointer(IMPORT, T, 'T', _RC)
call MAPL_GetPointer(IMPORT, Q, 'QV', _RC)
call MAPL_GetPointer(IMPORT, QL, 'QL', _RC)
call MAPL_GetPointer(IMPORT, QI, 'QI', _RC)
call MAPL_GetPointer(IMPORT, QR, 'QR', _RC)
call MAPL_GetPointer(IMPORT, QS, 'QS', _RC)
call MAPL_GetPointer(IMPORT, QG, 'QG', _RC)
call MAPL_GetPointer(IMPORT, RL, 'RL', _RC)
call MAPL_GetPointer(IMPORT, RI, 'RI', _RC)
call MAPL_GetPointer(IMPORT, RR, 'RR', _RC)
call MAPL_GetPointer(IMPORT, RS, 'RS', _RC)
call MAPL_GetPointer(IMPORT, RG, 'RG', _RC)
call MAPL_GetPointer(IMPORT, O3, 'O3', _RC)
call MAPL_GetPointer(IMPORT, CH4, 'CH4', _RC)
call MAPL_GetPointer(IMPORT, N2O, 'N2O', _RC)
call MAPL_GetPointer(IMPORT, CFC11, 'CFC11', _RC)
call MAPL_GetPointer(IMPORT, CFC12, 'CFC12', _RC)
call MAPL_GetPointer(IMPORT, HCFC22, 'HCFC22', _RC)
call MAPL_GetPointer(IMPORT, FCLD_IN, 'FCLD', _RC)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't these using ACG include file?

Comment on lines +661 to +678
call MAPL_GetPointer(INTERNAL, SFCEM_INT, 'SFCEM', _RC)
call MAPL_GetPointer(INTERNAL, FLX_INT, 'FLX', _RC)
call MAPL_GetPointer(INTERNAL, FLXA_INT, 'FLXA', _RC)
call MAPL_GetPointer(INTERNAL, FLC_INT, 'FLC', _RC)
call MAPL_GetPointer(INTERNAL, FLA_INT, 'FLA', _RC)
call MAPL_GetPointer(INTERNAL, FLXU_INT, 'FLXU', _RC)
call MAPL_GetPointer(INTERNAL, FLXAU_INT, 'FLXAU', _RC)
call MAPL_GetPointer(INTERNAL, FLCU_INT, 'FLCU', _RC)
call MAPL_GetPointer(INTERNAL, FLAU_INT, 'FLAU', _RC)
call MAPL_GetPointer(INTERNAL, FLXD_INT, 'FLXD', _RC)
call MAPL_GetPointer(INTERNAL, FLXAD_INT, 'FLXAD', _RC)
call MAPL_GetPointer(INTERNAL, FLCD_INT, 'FLCD', _RC)
call MAPL_GetPointer(INTERNAL, FLAD_INT, 'FLAD', _RC)
call MAPL_GetPointer(INTERNAL, TS_INT, 'TS', _RC)
call MAPL_GetPointer(INTERNAL, DFDTS, 'DFDTS', _RC)
call MAPL_GetPointer(INTERNAL, DFDTSC, 'DFDTSC', _RC)
call MAPL_GetPointer(INTERNAL, DFDTSNA, 'DFDTSNA', _RC)
call MAPL_GetPointer(INTERNAL, DFDTSCNA, 'DFDTSCNA', _RC)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use ACG?

Comment on lines +994 to +1005
real, pointer, dimension(:) :: PREF
real, pointer, dimension(:, :) :: TS
real, pointer, dimension(:, :) :: EMIS
real, pointer, dimension(:, :, :) :: PLE, T, Q, O3
real, pointer, dimension(:, :, :) :: CH4, N2O, CFC11, CFC12, HCFC22
real, pointer, dimension(:, :, :) :: QL, QI, QR, QS, QG
real, pointer, dimension(:, :, :) :: RI, RL, RR, RS, RG, FCLD_IN
real, pointer, dimension(:, :, :, :) :: RAERO
real, pointer, dimension(:, :, :) :: QAERO
real, pointer, dimension(:, :, :) :: CO2_3d => null() ! <<>> MSL
real, pointer, dimension(:, :, :) :: tmp_3d => null() ! <<>> MSL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACG?

@tclune tclune left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like important aspects of ACG usage are incomplete - see inline comments.

@pchakraborty

Copy link
Copy Markdown
Author

Looks like important aspects of ACG usage are incomplete - see inline comments.

Using the DeclarePointers and GetPointers headers generated by ACG will be the next step. I wasn't confident of getting it right in this pass, because of the several layers of nesting.

@tclune

tclune commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Looks like important aspects of ACG usage are incomplete - see inline comments.

Using the DeclarePointers and GetPointers headers generated by ACG will be the next step. I wasn't confident of getting it right in this pass, because of the several layers of nesting.

But ... you already did the biggest relevant change with is the variable declarations. And if there is some risk that the rest won't work we should not use the ACG at all. I prefer you get the entire ACG in place with this PR.

@pchakraborty

Copy link
Copy Markdown
Author

Looks like important aspects of ACG usage are incomplete - see inline comments.

Using the DeclarePointers and GetPointers headers generated by ACG will be the next step. I wasn't confident of getting it right in this pass, because of the several layers of nesting.

But ... you already did the biggest relevant change with is the variable declarations. And if there is some risk that the rest won't work we should not use the ACG at all. I prefer you get the entire ACG in place with this PR.

I did the easy change - something that makes comparing the MAPL2 and MAPL3 versions much easier - by removing the AddSpec calls in SetServices.

As for the rest of the ACG features, I'm a little more hesitant to include them in this PR. One of the first specific issues that comes to mind is retrieving a pointer to an export spec with ALLOC=.true.. I don't think it's worth adding a workaround in the MAPL2 version.

I also don't think skipping the Declare/GetPointers pieces for now makes a big difference in the maintainability of the MAPL version. The main maintainability win was removing the AddSpec calls, and creating the module level routines, and making the MAPL2/MAPL3 comparison simpler.

So my preference would be to keep this PR focused on the current changes and evaluate the remaining issues separately.

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

Labels

0 diff structural Structural changes to repository that are zero-diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants