Skip to content

Fixing output of nsformation and bhformation files#72

Merged
claireshiye merged 1 commit into
ClusterMonteCarlo:masterfrom
elenagonzalez870:kickflag_fixes
Mar 28, 2026
Merged

Fixing output of nsformation and bhformation files#72
claireshiye merged 1 commit into
ClusterMonteCarlo:masterfrom
elenagonzalez870:kickflag_fixes

Conversation

@elenagonzalez870

Copy link
Copy Markdown
Contributor
  • Definining pred_id1 and pred_id2 in do_stellar_evolution to prevent id=0 in nsformation and bhformation files. The reason for this is that within handle_bse_outcome (called before writing out to these files), an object can be destroyed if a merger happens, and hence the IDs in the binary[kb] structure are zeroed. I would recommned to move the writing to these files inside handle_bse_outcome such that there's more consistency.

  • Passing VKO as a pointer into handle_bse_outcome such that it gets updated when the net kicks are calculated. This bug caused neutron stars (NSs) and black holes (BHs) born in binaries in the nsformation and bhformation files to appear as though they did not receive kicks. The compact objects were receiving kicks; they were simply not written to the files correctly.

… id=0 in nsformation and bhformation files. The reason for this is that within handle_bse_outcome (called before writing out to these files), an object can be destroyed if a merger happens, and hence the IDs in the binary[kb] structure are zeroed. I would recommned to move the writing to these files inside handle_bse_outcome such that there's more consistency.

* Passing VKO as a pointer into handle_bse_outcome such that it gets updated when the net kicks are calculated. This bug caused neutron stars (NSs) and black holes (BHs) born in binaries in the nsformation and bhformation files to appear as though they did not receive kicks. The compact objects were receiving kicks; they were simply not written to the files correctly.
Copilot AI review requested due to automatic review settings March 26, 2026 17:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect/zeroed IDs and missing natal-kick values in nsformation/bhformation outputs by preserving pre-outcome IDs and plumbing the computed kick magnitude out of handle_bse_outcome.

Changes:

  • Pass VKO (kick magnitude) by pointer into handle_bse_outcome so callers can write the updated value to formation files.
  • Preserve binary[kb].id1/id2 into prev_id1/prev_id2 before handle_bse_outcome can zero IDs due to mergers/destruction.
  • Update formation-file writes to use preserved IDs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/cmc/cmc_stellar_evolution.c Preserves pre-outcome IDs and threads kick magnitude back to callers for correct NS/BH formation output.
include/cmc/cmc.h Updates handle_bse_outcome prototype to include double *VKO output parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 678 to +679
if (kprev1!=14 && binary[kb].bse_kw[1]==14 && binary[kb].id2 != 0) { // newly formed BH
parafprintf(newbhfile, "%.18g %g 1 %ld %g %g %g %g %g", TotalTime, star_r[g_k], binary[kb].id2, binary[kb].bse_zams_mass[1],binary[kb].bse_mass0[1], binary[kb].bse_mass[1], binary[kb].bse_bhspin[1],VKO);
parafprintf(newbhfile, "%.18g %g 1 %ld %g %g %g %g %g", TotalTime, star_r[g_k], prev_id2, binary[kb].bse_zams_mass[1],binary[kb].bse_mass0[1], binary[kb].bse_mass[1], binary[kb].bse_bhspin[1],VKO);

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

The guard conditions for the secondary (id2) still use binary[kb].id2 != 0, but the whole point of capturing prev_id2 is that handle_bse_outcome may zero binary[kb].id2. As written, you can still silently skip writing the formation line for star 2 even though you now print prev_id2. Use prev_id2 != 0 (or an equivalent pre-outcome validity check) in these if conditions to match the preserved-ID logic.

Copilot uses AI. Check for mistakes.
Comment on lines 692 to +693
if (kprev1!=13 && binary[kb].bse_kw[1]==13 && binary[kb].id2 != 0) { // newly formed NS
parafprintf(newnsfile, "%.18g %g 1 %ld %g %g %g %g %g %d", TotalTime, star_r[g_k], binary[kb].id2, binary[kb].bse_zams_mass[1],binary[kb].bse_mass0[1], binary[kb].bse_mass[1], binary[kb].bse_bcm_formation[1],VKO,kprev1);
parafprintf(newnsfile, "%.18g %g 1 %ld %g %g %g %g %g %d", TotalTime, star_r[g_k], prev_id2, binary[kb].bse_zams_mass[1],binary[kb].bse_mass0[1], binary[kb].bse_mass[1], binary[kb].bse_bcm_formation[1],VKO,kprev1);

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

The guard conditions for the secondary (id2) still use binary[kb].id2 != 0, but the whole point of capturing prev_id2 is that handle_bse_outcome may zero binary[kb].id2. As written, you can still silently skip writing the formation line for star 2 even though you now print prev_id2. Use prev_id2 != 0 (or an equivalent pre-outcome validity check) in these if conditions to match the preserved-ID logic.

Copilot uses AI. Check for mistakes.

@claireshiye claireshiye left a comment

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.

All looks correct.

@claireshiye
claireshiye merged commit 2e673ea into ClusterMonteCarlo:master Mar 28, 2026
4 of 5 checks passed
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.

3 participants