Skip to content

[Fix]: Separation of conditionals for selecting radiation in Physics call#98

Open
fmalatino wants to merge 2 commits into
NOAA-GFDL:developfrom
fmalatino:orch_rad
Open

[Fix]: Separation of conditionals for selecting radiation in Physics call#98
fmalatino wants to merge 2 commits into
NOAA-GFDL:developfrom
fmalatino:orch_rad

Conversation

@fmalatino
Copy link
Copy Markdown
Contributor

Description
This PR introduces changes to allow orchestration to parse the Physics object call, without entering into the block regarding radiation.

How Has This Been Tested?
Tested using current CI

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • Targeted model, if this change was triggered by a model need/shortcoming

@fmalatino fmalatino marked this pull request as ready for review May 4, 2026 21:12
Copy link
Copy Markdown
Collaborator

@oelbert oelbert left a comment

Choose a reason for hiding this comment

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

Great, thanks for fixing this Frank

@oelbert oelbert added this pull request to the merge queue May 5, 2026
@FlorianDeconinck
Copy link
Copy Markdown
Collaborator

Technical detail here: for orchestration the do_radiation is considered a runtime parameter, subject to change, it will therefore orchestrate through. The self._rterrtmgp on the other end is a None value will trigger a code cull and nothing will be orchestrated in the conditional branch

@oelbert
Copy link
Copy Markdown
Collaborator

oelbert commented May 5, 2026

Technical detail here: for orchestration the do_radiation is considered a runtime parameter, subject to change, it will therefore orchestrate through. The self._rterrtmgp on the other end is a None value will trigger a code cull and nothing will be orchestrated in the conditional branch

That's correct, do_radiation changes at runtime

@oelbert oelbert removed this pull request from the merge queue due to a manual request May 5, 2026
@oelbert oelbert self-requested a review May 5, 2026 13:51
Copy link
Copy Markdown
Collaborator

@oelbert oelbert left a comment

Choose a reason for hiding this comment

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

One small change, ideally we'd have the compile-time conditional before the runtime one

Comment thread pyshield/stencils/physics.py Outdated
Comment on lines +1630 to +1631
if do_radiation:
if self._rterrtmgp:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we reverse the order of these conditionals so the compile-time one is first?

@fmalatino fmalatino requested a review from oelbert May 5, 2026 15:35
Copy link
Copy Markdown
Collaborator

@oelbert oelbert left a comment

Choose a reason for hiding this comment

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

One more thing

self._level_flip,
)

self._interpolate_radiation(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be outside the if do_radiation condition

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants