Skip to content

Revert "Density-based θˡⁱ state: fix compressible saturation & temperature inversion (closes #765, supersedes #752)"#778

Draft
giordano wants to merge 1 commit into
mainfrom
revert-767-eq/compressible-density-thermo-state
Draft

Revert "Density-based θˡⁱ state: fix compressible saturation & temperature inversion (closes #765, supersedes #752)"#778
giordano wants to merge 1 commit into
mainfrom
revert-767-eq/compressible-density-thermo-state

Conversation

@giordano

Copy link
Copy Markdown
Member

Reverts #767.

I want to see if this resolves the slowdown in the Reactant tests observed at #773 (comment). CC @dkytezab @Pangoraw.

@giordano giordano added the reactant ☣ towards a differentiable earth label Jun 10, 2026
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@giordano

Copy link
Copy Markdown
Member Author

It was indeed #767, Reactant tests are back to normal times 😞

@glwagner

Copy link
Copy Markdown
Member

It is probably worth implementing a different solver that uses a fixed number of iterations, to see if this resolves the reactant performance issue. I am not surprised that a state dependent solver like a Newton solver induces a big slowdown -- we have seen that before

@giordano

Copy link
Copy Markdown
Member Author

The point is that compile time blows up, not simply runtime, XLA reports spending 25 minutes compiling in reactant/weno_compilation

@glwagner

Copy link
Copy Markdown
Member

The point is that compile time blows up, not simply runtime, XLA reports spending 25 minutes compiling in reactant/weno_compilation

Hmm, compile problems make sense, though I’m not sure what it has to do with WENO

@giordano

Copy link
Copy Markdown
Member Author

though I’m not sure what it has to do with WENO

The ways of XLA are infinite.

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

Labels

reactant ☣ towards a differentiable earth

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants