COSMIC-PopSynth / COSMIC

COSMIC (Compact Object Synthesis and Monte Carlo Investigation Code)
GNU General Public License v3.0
45 stars 58 forks source link

Allow user to specify a target total mass for independent sampler #602

Closed TomWagg closed 9 months ago

TomWagg commented 9 months ago

Hi! I mentioned to @katiebreivik that I was planning to make a sampler that targeted mass instead of size and she suggested I make a PR so here I am.

Big picture changes I made:

Some gritty details:

Testing Unsure exactly how testing is done with COSMIC but I figure I should add some simple unittests e.g.

  1. Sample size always >= size for sampling_target=size
  2. Sample mass always >= total_mass for sampling_target=total_mass
  3. Sample mass - total_mass <= 150 for sampling_target=total_mass I'll ask @katiebreivik about this tomorrow and add these to the PR ๐Ÿ™‚
TomWagg commented 9 months ago

Separately a question: The way the samples get masked seems overly complex to me but potentially missing something. But would it not work to turn this

        # select out the primaries and secondaries that will produce the final kstars
        (ind_select_primary,) = np.where(
            (mass1_binaries > primary_min) & (mass1_binaries < primary_max)
        )
        (ind_select_secondary,) = np.where(
            (mass2_binaries > secondary_min) & (mass2_binaries < secondary_max)
        )
        ind_select = list(
            set(ind_select_primary).intersection(ind_select_secondary))

into this?

        ind_select = ((mass1_binaries > primary_min)
                      & (mass1_binaries < primary_max)
                      & (mass2_binaries > secondary_min)
                      & (mass2_binaries < secondary_max))
codecov[bot] commented 9 months ago

Codecov Report

Attention: 301 lines in your changes are missing coverage. Please review.

Comparison is base (8772c07) 86.91% compared to head (8fc96a8) 87.73%. Report is 14 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #602 +/- ## =========================================== + Coverage 86.91% 87.73% +0.82% =========================================== Files 40 42 +2 Lines 25542 25687 +145 =========================================== + Hits 22198 22534 +336 + Misses 3344 3153 -191 ``` | [Files](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth) | Coverage ฮ” | | |---|---|---| | [cosmic/src/benchmarkevolv2.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy9iZW5jaG1hcmtldm9sdjIuZg==) | `100.00% <100.00%> (รธ)` | | | [cosmic/src/comenv.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy9jb21lbnYuZg==) | `35.05% <100.00%> (+1.03%)` | :arrow_up: | | [cosmic/src/hrdiag.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy9ocmRpYWcuZg==) | `81.53% <100.00%> (+42.41%)` | :arrow_up: | | [cosmic/evolve.py](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL2V2b2x2ZS5weQ==) | `55.19% <0.00%> (-0.23%)` | :arrow_down: | | [cosmic/utils.py](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3V0aWxzLnB5) | `80.27% <75.00%> (-0.03%)` | :arrow_down: | | [cosmic/sample/sampler/independent.py](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NhbXBsZS9zYW1wbGVyL2luZGVwZW5kZW50LnB5) | `84.10% <91.67%> (+15.90%)` | :arrow_up: | | [cosmic/src/hrdiag\_remnant.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy9ocmRpYWdfcmVtbmFudC5m) | `64.00% <64.00%> (รธ)` | | | [cosmic/src/zfuncs.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy96ZnVuY3MuZg==) | `56.54% <6.67%> (-11.68%)` | :arrow_down: | | [cosmic/src/assign\_remnant.f](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth#diff-Y29zbWljL3NyYy9hc3NpZ25fcmVtbmFudC5m) | `9.39% <9.39%> (รธ)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/COSMIC-PopSynth/COSMIC/pull/602/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=COSMIC-PopSynth)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

TomWagg commented 9 months ago

And finally here's a quick plot of how close I get to the target_mass when running 10000 samples, each targeting 15000 solar masses. Mostly pretty close but it all comes down to the IMF and if I get unlucky with a big binary right at the end image

TomWagg commented 9 months ago

@katiebreivik I've added the documentation that you suggested, let me know what you think! ๐Ÿ™‚ (I also spruced things up a little with some extra heading, sphinx admonitions and api refs - hope that's okay!)

katiebreivik commented 9 months ago

๐Ÿ‘