Skip to content

Conversation

nefrathenrici
Copy link
Member

@nefrathenrici nefrathenrici commented Jul 7, 2025

Purpose

Finish renaming variables after #3875. This is a breaking PR due to name changes within AtmosModel.

To-do

Content

Rename:

  • AtmosModel.vert_diff -> vertical_diffusion
  • AtmosModel.water.precip_model -> microphysics_model
  • PrognosticSurfaceTemperature -> SlabOceanSST
  • PrescribedSurfaceTemperature -> PrescribedSST

Note that all parsed args are still supported but `"prognostic_surface" = "prognostic/prescribedsurfacetemperature" is deprecated.


  • I have read and checked the items on the review checklist.

@nefrathenrici nefrathenrici force-pushed the ne/atmos_model branch 2 times, most recently from 9394a8a to 23a092a Compare July 7, 2025 18:50
@szy21
Copy link
Member

szy21 commented Jul 7, 2025

Could you also rename PrescribedSurfaceTemperature to PrescribedSST so it is more consistent?

@nefrathenrici
Copy link
Member Author

Could you also rename PrescribedSurfaceTemperature to PrescribedSST so it is more consistent?

Do you prefer PrescribedSST or PrescribedOceanSST?

@szy21
Copy link
Member

szy21 commented Jul 8, 2025

Could you also rename PrescribedSurfaceTemperature to PrescribedSST so it is more consistent?

Do you prefer PrescribedSST or PrescribedOceanSST?

I prefer PrescribedSST as it is shorter and clear enough to me, but if you prefer the other one, feel free to use it.

Copy link
Member

@tapios tapios left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

Copy link
Member

@trontrytel trontrytel left a comment

Choose a reason for hiding this comment

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

Thank you! Will take some time to get used to grepping for microphysics_model rather than precip_model ;) But I think the changes do improve the code readability.

Rename precip_model -> microphysics_model, don't change config arg

Rename PrognosticSurfaceTemperature -> SlabOceanSST, PrescribedSurfaceTemp to PrescribedSST
@nefrathenrici nefrathenrici enabled auto-merge July 10, 2025 16:55
@nefrathenrici nefrathenrici added this pull request to the merge queue Jul 10, 2025
Merged via the queue into main with commit 5b57e15 Jul 10, 2025
17 of 19 checks passed
@nefrathenrici nefrathenrici deleted the ne/atmos_model branch July 10, 2025 21:34
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.

4 participants