Skip to content

[fix/enhancement] Remove forced lower mass ratio limit #819

Open
maxbriel wants to merge 5 commits intov2.3from
mb_no_lower_q_limit
Open

[fix/enhancement] Remove forced lower mass ratio limit #819
maxbriel wants to merge 5 commits intov2.3from
mb_no_lower_q_limit

Conversation

@maxbriel
Copy link
Copy Markdown
Collaborator

@maxbriel maxbriel commented Mar 9, 2026

This PR contains PR #797 !

This PR removes the lower mass ratio limit that's forced by initial_sampling.
Additionally, it adds a warning when setting up a population synthesis run. if q<0.05 is sampled.

@maxbriel maxbriel requested a review from a team March 9, 2026 12:37
@maxbriel maxbriel self-assigned this Mar 9, 2026
@maxbriel maxbriel marked this pull request as ready for review March 27, 2026 12:33
synpop_params = binarypop_kwargs_from_ini(args.ini_file)
# warn if mass ratio q = M2/M1 could fall below 0.05
if synpop_params['secondary_mass_scheme'] == 'flat_mass_ratio':
q_min_possible = (synpop_params['secondary_mass_min']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, the q_min_possible should return a theoretically lowest case value, not a minimum from our actual sampling, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. Our sampling technically should never be below the theoretical minimum. I wanted to warn the user that their settings are allowing the sampler to return values below the grid values :)

@sgossage sgossage added the enhancement New feature or request label Apr 16, 2026
@maxbriel maxbriel added this to the v2.3 milestone Apr 24, 2026
@sgossage
Copy link
Copy Markdown
Contributor

As @maxbriel, pointed out, this PR currently has a failed assertion check in the unit tests that needs to be looked at more closely.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants