Skip to content

Reorganize lightcone generator functions#41

Open
nvillanova wants to merge 3 commits into
mainfrom
reorganize_lc_gens
Open

Reorganize lightcone generator functions#41
nvillanova wants to merge 3 commits into
mainfrom
reorganize_lc_gens

Conversation

@nvillanova

Copy link
Copy Markdown
Collaborator

This PR introduces a small refactor of the functions in diffhalos/lightcone_generators to make their outputs more consistent across the Monte Carlo and weighted implementations. In particular, the corresponding functions now return the same namedtuple structures, providing a unified interface.

This is particularly useful for the implementation of mc_lightcone_generators.py in diffsky, which will serve as the Monte Carlo counterpart to the existing wrapper around weighted lightcones.

Summary of the changes

  • mc_lightcone_halos.py : mc_lc_halos function now returns the exact same namedtuple CenPop as weighted_lc_halos , i.e., with the cen_weight field, which is equal to one for all halos.

  • mc_lightcone_subhalos.py : defined a global namedtuple SubPop . In order for the outputs to match in both mc_lc_subhalos and weighted_ld_subhalos functions , mc_lc_subhalos now computes some additional quantities.

  • mc_lightcone.py : both functions mc_lc and weighted_lc return the same new namedtuple HaloPop , which combines the information of centrals and subs.


__all__ = ("mc_lc_mf", "mc_lc", "weighted_lc")

# TODO: I think we could also define halo_weight == gal_weight = cen_weight * sat_weight here, instead of leaving the user to multiply afterwards.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree this would be nice to include. I think we will continue to need sat_weight and cen_weight separately, but including the product halo_weight is a nice convenience. Please add that, and include a test that the value stored in this new field is equal to cen_weight * sat_weight.

for i, _param in enumerate(mah_params_names):
mah_params_tot[i, :] = np.concatenate(
(
cenpop.mah_params._asdict()[_param],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is purely cosmetic: here I think it would be more elegant to use getattr(cenpop.mah_params, _param) rather than converting to a dict.

n_subs = len(logmu_obs)
sat_weight = jnp.ones(n_subs)
# TODO: check shape of sat_weight
# sat_weight = jnp.repeat(sat_weight, n_mu_per_host) ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: is this done? It's still listed as a TODO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants