Skip to content

Conversation

@ilyamandel
Copy link
Collaborator

No description provided.

Ilya Mandel added 2 commits June 25, 2025 15:11
Copy link
Collaborator

@jeffriley jeffriley left a comment

Choose a reason for hiding this comment

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

Approved with a couple of questions:

(i) on line 3911 you have the comment

"makes it possible to adjust if p_Rand is too low, to avoid getting stuck"

but on line 3914 you reduce rand. Does the comment correctly/completely capture what you mean?

(ii) should the threshold value of 1000.0 be a constant defined in constants.h (named something like DISBERG_MANDEL_MAX_KICK)? Then the 1001.0 in line 3910 can just be (e.g.) DISBERG_MANDEL_MAX_KICK + 1.0.

EDIT: also, rand = 0.99 * rand; can just be rand *= 0.99; (more succinct, and not too obscure...)

@pauldisberg
Copy link
Collaborator

image

I ran COMPAS v03.20.06 with ./COMPAS -n 100000 --mode SSE --kick-magnitude-distribution LOGNORMAL. From the results I selected the NS-forming SNe with SN_Type==1 and in the attached figure I plot a CDF of the values of "Applied_Kick_Magnitude" (which is exactly equal to "Drawn_Kick_Magnitude").

For v<900 km/s it exactly follows our log-normal distribution, if it is not normalized between 0 and 1000 km/s. Between 900 and 1000 km/s, there is a clear feature in the COMPAS results that makes them deviate from the log-normal and make the total distribution normalized for v<1000 km/s. Could it be that the threshold of 1000 km/s is currently causing all kicks with v>1000 km/s to be projected on the region between 900 and 1000 km/s? That is, in the PDF there is a clear peak between at v>900 km/s, so I believe something is not going right with the normalization. (This feature is, e.g., not present in the figure @SimonStevenson made in #1404.)

@pauldisberg
Copy link
Collaborator

pauldisberg commented Jun 26, 2025

image

Figure made with the updated v3.20.06 (similarly to the one described above, but with -n 10000 and both the LOGNORMAL and the MAXWELLIAN distributions). Looks as intended, approved.

@ilyamandel
Copy link
Collaborator Author

Thank you, @pauldisberg and @jeffriley .

I was also taking advantage of this PR to do a bit of work on the Matlab post-processing, which doesn't need to be reviewed. Done with that for the day, so merging in.

@ilyamandel ilyamandel merged commit 348395b into dev Jun 26, 2025
3 checks passed
@ilyamandel ilyamandel deleted the Matlab branch June 26, 2025 07:46
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.

4 participants