Skip to content

Implement differentiable satellite quenching#421

Merged
aphearin merged 34 commits into
mainfrom
gd_bugfix
May 29, 2026
Merged

Implement differentiable satellite quenching#421
aphearin merged 34 commits into
mainfrom
gd_bugfix

Conversation

@aphearin
Copy link
Copy Markdown
Collaborator

@aphearin aphearin commented May 13, 2026

This PR brings in a dev version of phot_kernels._mc_phot_kern. The implementation is in a new module, gd_phot_kernels.py. There is a unit test that ensures that gd_phot_kernels._mc_phot_kern and phot_kernels._mc_phot_kern return identical results for the stochastic obs_mags quantity. The gd_phot_kernels._mc_phot_kernfunction returns a bunch of additional quantities, includingobs_mags_weighted. The figure below shows that histograms of obs_magsandobs_mags_weighted` are very similar.

demo_obs_mags_weighted

@aphearin
Copy link
Copy Markdown
Collaborator Author

I have not made any attempt to optimize the new implementation, and so it is much slower and more memory intensive than the original module, because many redundant calculations are being done. I wanted to start with code-correctness before proceeding to optimization.

@aphearin
Copy link
Copy Markdown
Collaborator Author

A couple of scattered notes for future work on this PR:

First, a list of functions we should consider replacing:

  • ssp_weight_kernels.get_dust_randoms it would be more logical for this to instead be implemented in mc_randoms
  • sspwk.compute_burstiness is now obsolete and the bug-fixed code now calls sspwk.get_burstiness, so we should probably outright delete sspwk.compute_burstiness. However, there is still some considerable refactoring that will need to be done to improve efficiency of these implementations, so this can wait for now.
  • Same goes for sspwk.compute_diffstar_info- the corrected code is now based on sspwk.get_diffstar_info.

Separately, diffmerge is based on t_infall, whereas diffstarpop is based on gyr_since_infall = t_obs-t_infall This is a likely source of future confusion/bugs. Fortunately, these variable names are very clear to interpret, and I think I have consistently used them throughout the code. But this should be double-checked before we merge this eventual PR into main. And we should at least consider re-implementing these models so that the user-facing functions in each model accept the same convention.

@aphearin aphearin changed the title WIP: Develop differentiable version of _mc_phot_kern Implement differentiable satellite quenching May 29, 2026
@aphearin
Copy link
Copy Markdown
Collaborator Author

The code in this PR has been used by both @zaidikumail and @MitraKaustav in gradient descents, and the rapid quenching implementation has been stress-tested by @zaidikumail, so I'll proceed to merge this into main, and cut a new minor release so that we have a tagged version of the code with science validation tests.

@aphearin
Copy link
Copy Markdown
Collaborator Author

Note that #424 was a closely related investigation to this PR, I think maybe we should close #424 now that we found a better solution to the blue merging problem, but @zaidikumail should confirm before we do that.

@aphearin aphearin merged commit ec7c7ec into main May 29, 2026
11 checks passed
@aphearin aphearin deleted the gd_bugfix branch May 29, 2026 21:27
@zaidikumail
Copy link
Copy Markdown
Collaborator

Note that #424 was a closely related investigation to this PR, I think maybe we should close #424 now that we found a better solution to the blue merging problem, but @zaidikumail should confirm before we do that.

Yes, we can close #424

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