BUG: Use cm_per_mpc instead of 3.08e24 #3867

[lconaboy]

Fix for #3867.

PR Summary

Use cm_per_mpc instead of 3.08e24 for conversion from Mpc to cm. Also partly addresses #2098:

ds = yt.load('output_00080/info_00080.txt')
tA = ds.current_time.to('Myr')
tB = ds.cosmology.t_from_z(ds.current_redshift).to('Myr')

print(tA-tB)

where tA-tB is now -0.5628994599301222 Myr where it was -22.47323433951169 Myr.

[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!

[neutrinoceros]

I'll leave triage to @cphyc: don't hesitate to put this in the 4.0.3 milestone if it makes sense to you

[cphyc]

Thanks for the contribution!

[lconaboy]

Looks like a test uses the old value of 3.08e24 to calculate the compared simulation time

 Age simu (yt old const)=   1.1192112175127416     
 Age simu (yt new const)=   1.1212743045965676    

and hence this commit fails. One option is to update the test values, but since this test is on an old output (probably made before the introduction of the more accurate Mpc2cm constant into RAMSES via amr/constants.f90) maybe that isn't appropriate (at least not without warning people).

[neutrinoceros]

Good catch. Is it possible to parse (or deduce) which version of RAMSES produced a given output file ? If not, I'd err on the side of embracing the newest version rather than the oldest, but I guess we'd need to expose some parameter to make it possible to reproduce the old behaviour.

[cphyc]

Unfortunately, I don't think there is any easy way of telling the RAMSES version where this change was introduced, so I'd be in favour of accepting this PR as is (and update the failing test accordingly).

[lconaboy]

I agree about there being no way to tell which version. Do you think this needs some kind of warning?

[neutrinoceros]

If we're going to support only one convention I agree that it's best to go with the newest. Do you guys have an idea when it was switched in RAMSES ?

[lconaboy]

As far as I can tell, it was introduced to master around November 2018, and is in the stable_19_09 and stable_19_10 branches.

[neutrinoceros]

Ok. Sounds old enough that I'd side with Corentin: let's just update the existing test result.

[neutrinoceros]

pre-commit.ci autofix

[neutrinoceros]

LGTM, but I'll let @cphyc push the button :)

[welcome]

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