Skip to content

Update diffstarpop defaults to agree with DIFFSTARPOP_FITS_SMDPL_DR1_DIFFSTARPOP_PARAMS - #99

Merged
aphearin merged 5 commits into
mainfrom
update_defaults
Oct 21, 2025
Merged

Update diffstarpop defaults to agree with DIFFSTARPOP_FITS_SMDPL_DR1_DIFFSTARPOP_PARAMS#99
aphearin merged 5 commits into
mainfrom
update_defaults

Conversation

@aphearin

Copy link
Copy Markdown
Contributor

Update diffstarpop defaults to agree with DIFFSTARPOP_FITS_SMDPL_DR1_DIFFSTARPOP_PARAMS

@alexalar please double-check me that I've made the correct choice for what we want our default model behavior to be. All I did was copy-and-paste the parameter values from params.params_diffstarpopfits_mgash_smdpl_dr1.SFH_PDF_QUENCH_PARAMS into kernels.sfh_pdf_mgash.SFH_PDF_QUENCH_PDICT. I have made no changes to the quenching parameters, which are the same as whatever is stored in satquenchpop_model.DEFAULT_SATQUENCHPOP_PARAMS

… params_diffstarpopfits_mgash_smdpl_dr1.SFH_PDF_QUENCH_PDICT
@aphearin
aphearin requested a review from alexalar October 13, 2025 18:37

@alexalar alexalar left a comment

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.

@aphearin These parameters include in-plus-ex-situ SFH. Do we want to instead use the in-situ SFH params from params_diffstarpopfits_mgash_smdpl_dr1_nomerging.py?

And yes, the satquenchpop_model.DEFAULT_SATQUENCHPOP_PARAMS are not really used currently, since we are not taking into account the host halo mass nor the time of infall.

@aphearin

Copy link
Copy Markdown
Contributor Author

@alexalar when I switch best-fit parameters to smdpl_dr1_nomerging, all of a sudden we get a test failure for the test_all_diffstarpop_u_param_gradients_are_nonzero test in diffstar/diffstarpop/tests/test_gradients.py. You can see the traceback in the PR check, but here is the offending parameter:

AssertionError: ('Parameters with exact zero gradients:', [np.str_('u_lgmu_lo_mh_lo')])

I rewrote this test to make it a little easier to test any of our pre-fit models, and the only model for which this test fails is smdpl_dr1_nomerging.

I think maybe this is harmless because this parameter controls μ-dependent behavior. What do you think is the best way to handle this for present purposes? Should we modify the test to ignore satellite-specific parameters? Or does this indicate an actual problem that we should look into?

…ients to the parameter lgmu_lo_mh_lo parameter
@alexalar

alexalar commented Oct 16, 2025

Copy link
Copy Markdown
Collaborator

@aphearin Currently this module (diffstarpop.kernels.satquenchpop_model) is not being used in any of the calibrations. I'm not sure right now why changing the diffstarpop.kernels.sfh_pdf_mgash default parameters would now make any test fail.

What I have noticed is that there seems to be no real problem with the gradients. The gradients are not really zero, just very small (1e-14), which is below our absolute tolerance in the test of 1e-13. Upon further inspection, the u_lgmu_lo_mh_lo parameter controls the lower value of a sigmoid that transitions very fast. The fact that the transition is very fast means that very few haloes are affected meaningfully by a change of u_lgmu_lo_mh_lo. One option is to change the transition speed of this sigmoid for now, so this test passes, which is what I have done.

@aphearin

Copy link
Copy Markdown
Contributor Author

Great, thanks for looking into this. Seems like a reasonable fix for present purposes. How about we proceed to merge this then?

@aphearin
aphearin merged commit 95495b8 into main Oct 21, 2025
6 checks passed
@aphearin
aphearin deleted the update_defaults branch October 21, 2025 18:13
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