Skip to content

Conversation

@haakon-e
Copy link
Member

@haakon-e haakon-e commented Nov 25, 2025

This pull request primarily refactors and cleans up the code in the surface conditions and surface flux modules, focusing on improving readability and maintainability without changing core logic. The changes involve consolidating multi-line expressions into single lines, improving destructuring and broadcasting, and removing unnecessary variables. There are no significant algorithmic or behavioral changes.

Key refactoring and cleanup changes:

Surface conditions module (src/surface_conditions/surface_conditions.jl):

  • Consolidated multi-line variable assignments and function calls into single lines throughout, improving readability [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12].
  • Improved destructuring of external forcing inputs for clarity.
  • Used cospi for surface temperature calculation, simplifying trigonometric expressions.
  • Removed unused variables and unnecessary comments for conciseness.
  • Cleaned up struct construction and field assignment for surface conditions [1] [2].

Surface state module (src/surface_conditions/surface_state.jl):

  • Reformatted struct definitions and constructors to single lines for conciseness [1] [2] [3] [4].

Surface flux tendency (src/prognostic_equations/surface_flux.jl):

  • Refactored calculation of total specific enthalpy for clarity and efficiency.

These changes are focused on code style and maintainability, making the codebase easier to read and modify without altering its functionality.

@haakon-e haakon-e changed the base branch from he/rft-clean-up-diagnostics to main November 25, 2025 21:57
@haakon-e haakon-e force-pushed the he/rft-clean-surface-code branch from da52bea to 86340ad Compare November 25, 2025 21:57
),
)
ᶜe_tot = @. lazy(specific(Y.c.ρe_tot, Y.c.ρ))
ᶜh_tot = @. lazy(TD.total_specific_enthalpy(thermo_params, ᶜts, ᶜe_tot))
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this change is necessary, but I am ok with it (we have the same ᶜh_tot calculation in a lot of other places and it would be good to be consistent).

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that it's not necessary, but I think it makes the code more readable. Happy to change back if you prefer the existing way though (I'm also happy to change this everywhere else for the sake of consistency)

Copy link
Member

Choose a reason for hiding this comment

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

ok let's change here and elsewhere then, thanks!

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