Skip to content

Fixing satquench model - #116

Merged
aphearin merged 2 commits into
mainfrom
fixing_satquench
May 5, 2026
Merged

Fixing satquench model#116
aphearin merged 2 commits into
mainfrom
fixing_satquench

Conversation

@alexalar

@alexalar alexalar commented May 5, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes a bug in the implementation of the additional satellite quenching model.

Currently, if a halo is a central, the satellite quenching model should have no effect due to the values of the variables lgmu_infall,logmhost_infall,gyr_since_infall, for that halo. However, when the halo is a satellite, then the module incorrectly updates the quenched fraction probability of centrals, which will not be used because the halo is a satellite.

@aphearin

aphearin commented May 5, 2026

Copy link
Copy Markdown
Contributor

We currently have one failing test: test_gradients_of_diffstarpop_pdf_satquench_params_are_nonzero. However, when I run this test locally on my laptop, it passes. Not sure what's going in here.

@alexalar

alexalar commented May 5, 2026

Copy link
Copy Markdown
Collaborator Author

I think this test should be failing with the update, because it computes the gradients for the mse loss of frac_quenc_cen, which now is no longer affected by satquench parameters. It should instead calculate now the mse loss of frac_quenc_sat. Let me correct it. It is strange if it wasn't failing for you locally.

@aphearin

aphearin commented May 5, 2026

Copy link
Copy Markdown
Contributor

Great, that was going to be tedious for me to figure out - thanks for the quick fix! Looks like your most recent commit did the trick.

@aphearin

aphearin commented May 5, 2026

Copy link
Copy Markdown
Contributor

However, when I run this test locally on my laptop, it passes. Not sure what's going in here.

Ohh and I just realized that when I tested this locally, I failed to check out the fixing_satquench branch, instead I incorrectly ran this test using the main branch. So it makes sense it it passed for me locally.

@aphearin aphearin mentioned this pull request May 5, 2026
@aphearin
aphearin merged commit 8d072a6 into main May 5, 2026
6 checks passed
@aphearin
aphearin deleted the fixing_satquench branch May 5, 2026 20:26
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