Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify handling of conformal birth times in RAMSES #4706

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

cphyc
Copy link
Member

@cphyc cphyc commented Oct 17, 2023

RAMSES ships with a utility to convert the particle creation times from conformal units into Gyr. This creates one file named birthXXXX.outYYYYY, where XXXXX is the number of the output and YYYYY is the index of the computation domain.

This PR detects those files and reads the birth times from there rather than computes them from the conformal time.

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

@matthewturk
Copy link
Member

In general, I'm inclined to accept this. Do you have an idea what's up with the tests?

@neutrinoceros
Copy link
Member

Do you have an idea what's up with the tests?

Logs are lost, let's try again: @yt-fido test this please

@cphyc
Copy link
Member Author

cphyc commented Nov 2, 2023

Before we merge this, I'd like to backport the changes I made in pynbody/pynbody#740.

@neutrinoceros
Copy link
Member

@cphyc could you select appropriate labels for this ? I don't know wether this is a pure refactor or an enhancement.

@cphyc cphyc added enhancement Making something better refactor improve readability, maintainability, modularity labels Nov 2, 2023
@cphyc cphyc added this to the 4.4.0 milestone Apr 12, 2024
@cphyc
Copy link
Member Author

cphyc commented May 17, 2024

Well, this is both a refactor and an enhancement :)

@cphyc cphyc added code frontends Things related to specific frontends backwards incompatible This change will change behavior labels May 17, 2024
@cphyc cphyc marked this pull request as ready for review May 17, 2024 15:40
@cphyc cphyc marked this pull request as draft May 17, 2024 15:40
@cphyc
Copy link
Member Author

cphyc commented May 17, 2024

Welp, it looks like the following fields appear broken with RAMSES:

@matthewturk
Copy link
Member

@cphyc do you need a hand?

@cphyc
Copy link
Member Author

cphyc commented May 23, 2024

@yt-fido test this please

@cphyc cphyc marked this pull request as ready for review May 23, 2024 14:58
@cphyc
Copy link
Member Author

cphyc commented May 24, 2024

OK, now that #4880 is fixed, I think everything is working as intended now (finally)!

import yt
import numpy as np

@yt.particle_filter(requires=["conformal_birth_time"])
def star(pfilter, data):
    return data[pfilter.filtered_type, "conformal_birth_time"] != 0

@yt.particle_filter(requires=["conformal_birth_time"])
def DM(pfilter, data):
    return data[pfilter.filtered_type, "conformal_birth_time"] == 0

ds = yt.load(
    "output_00080",
    extra_particle_fields=[
        ("particle_birth_time", "d"),
        ("particle_metallicity", "d"),
    ],
)
ds.add_particle_filter("star")
ds.add_particle_filter("DM")
ad = ds.all_data()

print(f"{ds.current_time.to("Gyr")=}")
print(ad["io", "age"].min().to("Gyr"), ad["io", "age"].max().to("Gyr"))
print(ad["io", "particle_birth_time"].min().to("Gyr"), ad["io", "particle_birth_time"].max().to("Gyr"))
print(ad["io", "star_age"].min().to("Gyr"), ad["io", "star_age"].max().to("Gyr"))

This returns:

ds.current_time.to("Gyr")=unyt_quantity(11.92528501, 'Gyr')
0.0 Gyr 11.925285011256845 Gyr
0.0 Gyr 11.925285011256845 Gyr
1.7763568394002505e-15 Gyr 11.225119034912565 Gyr

So all fields now seem to be properly defined (the 1e-15Gyr in the star_age is due to interpolation).


Regarding star_age, it looks quite reasonable if e.g. I make a plot of the star formation history of the simulation.

import matplotlib.pyplot as plt
mask = ad["io", "conformal_birth_time"] != 0
tbins = np.arange(0, ds.current_time.to("Gyr").value, 0.1)
Mstar, _ = np.histogram(
    ad["star", "star_age"].to("Gyr").value,
    weights=ad["star", "particle_mass"].to("Msun").value,
    bins=tbins,
)
plt.step(
	tbins[1:],
	Mstar / np.diff(tbins) / 1e9,
)
plt.yscale("log")
plt.xlabel(r"Lookback time [$\mathrm{Gyr}$]")
plt.ylabel(r"SFR [$M_\odot/\mathrm{yr}$]")
plt.savefig("/tmp/SFR.png")

SFR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible This change will change behavior code frontends Things related to specific frontends enhancement Making something better refactor improve readability, maintainability, modularity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants