[BUG] Fix missing mu in temperature calculation RAMSES

[V-Nathir]

PR Summary

The YT´s output for Temperature was not exactly the temperature, it was temperature over mu. This mu is required since P/rho = kb/mu * T/m_p; where p=pressure, rho=density, T=Temperature, m_p=proton mass, kb=Boltzmann Cte., and mu is the dimensionless average particle weight. Yt used to do P/rho * mp/kb = T, this T is now T_over_mu, and the temperature is now T_over_mu * mu. Also, the dimension of mu has been changed.

PR Checklist

• New features are documented, with docstrings and narrative docs
• Adds a test for any bugs fixed. Adds tests for new features.

[welcome]

Hi! Welcome, and thanks for opening this pull request. We have some guidelines for new pull requests, and soon you'll hear back about the results of our tests and continuous integration checks. Thank you for your contribution!

[cphyc]

Thanks for the PR @V-Nathir! I made some modifications to ensure that we are backwards compatible with old code versions. If we cannot read the cooling tables, we assume that T = T/µ (it is wrong, but we can't easily get µ) and we raise a warning when the user tries to access the temperature. For recent code versions, we interpolate µ from the cooling tables, T/µ and nH.

For reviewers: this is a breaking change but also a bug. I don't really see a way forward to fixing it that would satisfy these two constrains:

1. the same code provides the same output as pre-bug versions,
2. new users get the right temperature, i.e. we do not store the "fixed" version in a different field name, like "temperature_that_is_T_rather_than_T/mu"

[cphyc]

Failure on the build on py3.12 is unrelated :)

[neutrinoceros]

How about we just ship it in our next feature release instead of a bugfix release ?

[cphyc]

Either way is fine by me.

[cphyc]

This looks good to me, but I'd like a second pair of eyes with someone from astro (@brittonsmith?)

[neutrinoceros]

Either way is fine by me.

Then I should leave this choice to reviewers. Please remember to set a milestone before you merge this. Thanks !

[cphyc]

The failing tests are unrelated and fixed in #4819.

The base branch was changed.

[neutrinoceros]

@cphyc I trust Britton's approval and am ready to hit the button, but first I'd like us to clarify why you marked this change as "backward incompatible": what I think I understand here is that outputs from yt 4.3.1 were incorrect, so changing them is not bug-for-bug backwards compatible, sure, but it wouldn't break any downstream correct code, would it ?

[cphyc]

It depends what you define as buggy. We've been calling temperature $T/\mu$ since the very first commit supporting RAMSES datasets in yt. This PR fixes this by setting the definition of the temperature to be $T$ instead, and temperature_over_mu is the old definition.

In this context, $\mu$ is the mean molecular weight. It is of order ~1, but is temperature dependent.

[neutrinoceros]

Ok, I'll make sure this is explicitly called out in release notes then. For the sake of refreshing CI, can you rebase or merge from main again ?

[neutrinoceros]

re-approving by proxy

[welcome]

Hooray! Congratulations on your first merged pull request! We hope we keep seeing you around! 🎆