GAMER frontend modification for incorporating lookback time in cosmological simulation

[koarakawaii]

Hi, this is a GAMER frontend modification for the current_time definition for GAMER cosmology simulation, thank you!

PR Summary

• Modify definition for current_time in GAMER cosmology simulation (COMOVING is on)
  • Before modification, current_time equals to scale factor a, resulting in problematic value when annotating timestamp.
  • After modification, current_time equals to cosmological age at given redshift.
• Definition for current_time is intact for COMOVING is off
• Add explanation for code lines

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

Hi, thank you for this contribution !
This looks like a bugfix to me but I'll defer to more expert maintainers to validate that it's correct.

[koarakawaii]

Hi, @neutrinoceros I have tried to fix the typos and errors you mentioned and detected by the pre-commit.ci. Thank you for pointing it out!

[jzuhone]

@koarakawaii So in all of the other frontends in yt which use cosmology, the current_time attribute refers to the time since the beginning of the universe (whether as the scale factor or something else) and not the lookback time. For consistency across codes it may make more sense to do this in the GAMER code frontend.

It also may make sense to give annotate_time an option to use the lookback time instead of the time since the beginning of the universe. I'd be happy to help do that if you're interested in it.

[koarakawaii]

Hi @jzuhone:
Thank you for the suggestion and comment~

@koarakawaii So in all of the other frontends in yt which use cosmology, the current_time attribute refers to the time since the beginning of the universe (whether as the scale factor or something else) and not the lookback time. For consistency across codes it may make more sense to do this in the GAMER code frontend.

I think I might make the title a little bit confusing. I guess despite the function (yt.utilities.cosmology.lookback_time) we used is named by "lookback time", it actually can compute the elapsed time between two given redshift z_i (lower redshift) and z_f (higher redshift), as suggested by the function prototype here. And we set z_i as the self.current_redshift for a given simulation snapshot, while z_f = 1E6 (very early universe), which is same as GADGET frontend, so it seems to me the outcome is approximately the elapsed time from the beginning of the universe (hope I did not confused anything).

It also may make sense to give annotate_time an option to use the lookback time instead of the time since the beginning of the universe. I'd be happy to help do that if you're interested in it.

As mentioned above, our implementation will use the elapsed time from the beginning of universe to a given redshift as annotate_time, but as you said, it might be helpful to allow the freedom for choosing between using age of universe or lookback time from now as the annotate_time. Just want to know is there an appropriate way for implementing this~? Since this is my very first PR to yt so I would like to know the proper way for contributing to this repository 😅

[jzuhone]

Hi @koarakawaii, do we have a small cosmological dataset we can test with? No worries if not.

[koarakawaii]

@jzuhone I think we have some CDM cosmology simulation snapshots from z = 100 to z = 0, with 128^3 particles in a 30 Mpc/h box (initial condition generated by MUSIC). Will they help?

[jzuhone]

If that's something you can share, definitely.

[koarakawaii]

@jzuhone I'll see whether I can upload them to yt Hub or somewhere else~

[koarakawaii]

@jzuhone We prepare a GAMER CDM snapshot at z~0, with 128^3 particles inside a 30 Mpc/h box, with uniform resolution 128X128X128. The data can be accessed via: curl -JO http://use.yt/upload/84a933ad. If you need more data please kindly inform us. Thank you!

[jzuhone]

@koarakawaii sorry for the delay. I will check this dataset today. Is it ok if we post it to https://yt-project.org/data? We also need to add some tests for cosmological functionality to yt/frontends/gamer/tests/test_outputs.py, but not necessarily with this PR.

[koarakawaii]

Hi, @jzuhone :

Is it ok if we post it to https://yt-project.org/data?

I think is OK to post it to https://yt-project.org/data~ But I might need some time to figure out how to do it:sweat_smile: Is there a tutorial which I can follow~?

We also need to add some tests for cosmological functionality to yt/frontends/gamer/tests/test_outputs.py, but not
necessarily with this PR.

Got you. Sounds great!

[neutrinoceros]

But I might need some time to figure out how to do it😅 Is there a tutorial which I can follow~?

This isn't something you have permissions for, all we need is your approval 😄

[neutrinoceros]

sorry for closing/reopening, my finger slipped

[koarakawaii]

Hi @neutrinoceros:
Thank you for the clarification! I upload the same dataset but with filename changed, which I guess can make its content more self-explained. The new data can be accessed via curl -JO http://use.yt/upload/342ec4e0, and we agree to post it to https://yt-project.org/data . And if possible, can we delete the dataset I uploaded before, with hash 84a933ad~? Thank you in advance!

[neutrinoceros]

@jzuhone Unless you need a specific second reviewer, I think you can merge. Just one question: would this be a candidate for backporting if we were to do another bugfix release ?

[welcome]

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