Skip to content

Add NewtonsMethod for all thermo types#308

Merged
szy21 merged 1 commit intomainfrom
ts/sa-maxiter-default
Feb 5, 2026
Merged

Add NewtonsMethod for all thermo types#308
szy21 merged 1 commit intomainfrom
ts/sa-maxiter-default

Conversation

@tapios
Copy link
Member

@tapios tapios commented Feb 2, 2026

No description provided.

@tapios tapios requested a review from szy21 February 2, 2026 19:46
Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

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

Could you change it for ::ph too? And I think you want tol = 0 instead of nothing? And given that it is with a fixed iteration, maybe we can return without converged, so we don't need those wrappers in ClimaAtmos?

@szy21
Copy link
Member

szy21 commented Feb 2, 2026

Actually I think we should change the default in the convenience method, not the actual function.

@szy21 szy21 force-pushed the ts/sa-maxiter-default branch 8 times, most recently from 6150f3a to 9870f3b Compare February 3, 2026 08:12
@szy21 szy21 changed the title Add maxiter default, bump patch Add NewtonsMethod for all thermo types Feb 3, 2026
@szy21 szy21 force-pushed the ts/sa-maxiter-default branch 2 times, most recently from da35602 to 200cbd4 Compare February 3, 2026 21:57
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 87.37201% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.58%. Comparing base (93162a7) to head (dc450ce).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/saturation_adjustment.jl 87.37% 37 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
- Coverage   87.37%   86.58%   -0.79%     
==========================================
  Files          22       22              
  Lines        1156     1357     +201     
==========================================
+ Hits         1010     1175     +165     
- Misses        146      182      +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@szy21 szy21 force-pushed the ts/sa-maxiter-default branch from 200cbd4 to dedfecf Compare February 4, 2026 06:16
Copy link
Member Author

@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.

Consider reducing duplication by consolidating the fixed_iter functions into one function with methods for the variable combinations.

It would also be good to add tests for the derivatives (against finite differences) to ad_tests.jl.

@szy21 szy21 force-pushed the ts/sa-maxiter-default branch 4 times, most recently from d6c838d to 764eed1 Compare February 5, 2026 00:35
@szy21 szy21 force-pushed the ts/sa-maxiter-default branch from 6f81a2c to dc450ce Compare February 5, 2026 01:07
@szy21 szy21 merged commit 6fb60b4 into main Feb 5, 2026
18 checks passed
@tapios tapios deleted the ts/sa-maxiter-default branch February 5, 2026 18:26
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.

2 participants