Fix definition of particle age/times

[cphyc]

PR Summary

This fixes #3861.

[matthewturk]

This looks alright to me. I am not sure I can properly 'approve' however.

[neutrinoceros]

Agreed. Let's wait on the reporter's approval for this one.

[lconaboy]

Hi @cphyc, this looks good. Just tested on my dataset and compared the results to those produced by the ramses utils. The birth times/ages match to similar accuracy as I mentioned in #3861 -- most of the ages agree to +/- 0.005 Myr, birth times are a little off, with most of them very close to -0.7 Myr compared to the ramses utils. I don't think this is due to this PR, though, instead I think it is down to the discrepancy between ds.current_time as calculated yt (I'm getting 381.03 Myr) and time_simu as calculated by the ramses utils (I'm getting 381.75 Myr). Not sure why that arises, but it's not due to this (as far as I can tell).

[cphyc]

Hi @cphyc, this looks good. Just tested on my dataset and compared the results to those produced by the ramses utils. The birth times/ages match to similar accuracy as I mentioned in #3861 -- most of the ages agree to +/- 0.005 Myr, birth times are a little off, with most of them very close to -0.7 Myr compared to the ramses utils. I don't think this is due to this PR, though, instead I think it is down to the discrepancy between ds.current_time as calculated yt (I'm getting 381.03 Myr) and time_simu as calculated by the ramses utils (I'm getting 381.75 Myr). Not sure why that arises, but it's not due to this (as far as I can tell).

The small discrepancy come from the way we interpolate times from conformal to physical, which is probably slightly different (though I wouldn't know which one of RAMSES' and yt's is the most accurate).

[neutrinoceros]

Great, thanks @iconaboy for your feedback.
@cphyc is this ready for merging ?

[cphyc]

@neutrinoceros yes!

[lconaboy]

The small discrepancy come from the way we interpolate times from conformal to physical, which is probably slightly different (though I wouldn't know which one of RAMSES' and yt's is the most accurate).

Ah ok, I thought it might be something like this.

I dug a bit and just noticed that in yt/frontends/ramses/data_structures.py, the value 3.08e24 is used for cm_per_mpc, from line 974:

                self.current_time = (
                    (self.time_tot + self.time_simu)
                    / (self.hubble_constant * 1e7 / 3.08e24)
                    / self.parameters["unit_t"]
                )

I think this is a hangover from when ramses used to use 3.08e24 as the conversion from Mpc to cm, but newer versions use the more accurate 3.0856776d+24 (this is the value I used for the ramses utils comparison), which matches up with the yt value in yt/utilities/physical_ratios.py. Swapping the line

(self.hubble_constant * 1e7 / 3.08e24)

for

(self.hubble_constant * 1e7 / cm_per_mpc)

brings most of the particle_birth_time values to within +/- 0.01 Myr of the ramses utils (though there is some tail still, due to interpolation?)

This is a separate issue to the one @cphyc fixed above, though, might be better if I make a new issue/PR?

e: Might this inconsistency in cm_per_Mpc be related to #2098 @cphyc?
ee: Somewhat, though there is still some error (again, from interpolation?). Using cm_per_Mpc gives (as in #2098) tA-tB = -0.562655331151916 Myr whereas 3.08e24 gives tA-tB = -1.2650417797501063 Myr where tA is ds.current_time.to('Myr') and tB is ds.cosmology.t_from_z(ds.current_redshift).to('Myr').

[cphyc]

@lconaboy that may indeed be related to #2098. If you want to submit a PR changing 3.08e24 to cm_per_mpc that would be a great first contribution :)