Skip to content

Conversation

@lwJi
Copy link
Collaborator

@lwJi lwJi commented Oct 21, 2025

PunctureTracker has array of 10 elements to store punctures by default.
Without READS, those inactive elements will be set to nan by the poison mechanism.

@lwJi lwJi requested a review from rhaas80 October 21, 2025 15:38
@lwJi
Copy link
Collaborator Author

lwJi commented Oct 22, 2025

@rhaas80 I removed the change in schedule.ccl and write those inactive elements instead to workaround CarpetX poison mechanism

@rhaas80
Copy link
Member

rhaas80 commented Oct 22, 2025

I took a look at BoxInBox's interface.ccl and it reads:

CCTK_REAL positions[3] TYPE=scalar
{
  position_x position_y position_z
} "Positions of refined regions"

meaning that writing to any position_x[n] with n>2 will cause a SEGFAULT (or at least write to memory it should not write to).

This is a general bug with the code in PunctureTracker.

Since it's not in the ET, I officially don't care. However it cannot be added to the ET in this form yet.

Copy link
Member

@rhaas80 rhaas80 left a comment

Choose a reason for hiding this comment

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

it's a horrible workaround and cannot in this form be merged into the official ET code base.

In a private repo though: it will get the job done.

@lwJi
Copy link
Collaborator Author

lwJi commented Oct 22, 2025

I took a look at BoxInBox's interface.ccl and it reads:

CCTK_REAL positions[3] TYPE=scalar
{
  position_x position_y position_z
} "Positions of refined regions"

meaning that writing to any position_x[n] with n>2 will cause a SEGFAULT (or at least write to memory it should not write to).

This is a general bug with the code in PunctureTracker.

Since it's not in the ET, I officially don't care. However it cannot be added to the ET in this form yet.

What do you mean by 'a general bug with PunctureTracker'? If it is, I can fix it.

@rhaas80
Copy link
Member

rhaas80 commented Oct 22, 2025

What do you mean by 'a general bug with PunctureTracker'? If it is, I can fix it.

PunctureTracker currently does:

    for (int n = 0; n < nPunctures; ++n) {
      CCTK_VINFO("Writing punc coords to box %d.", n);
      position_x[n] = location[0][n];
      position_y[n] = location[1][n];
      position_z[n] = location[2][n];
    }

so if nPunctures > 2 it will write to non-existent position_x elements. As written right now, nPunctures must be at most 3 (and currently it could go up to 10 given PunctureTracker's param.ccl, can it?).

@lwJi
Copy link
Collaborator Author

lwJi commented Oct 22, 2025

What do you mean by 'a general bug with PunctureTracker'? If it is, I can fix it.

PunctureTracker currently does:

    for (int n = 0; n < nPunctures; ++n) {
      CCTK_VINFO("Writing punc coords to box %d.", n);
      position_x[n] = location[0][n];
      position_y[n] = location[1][n];
      position_z[n] = location[2][n];
    }

so if nPunctures > 2 it will write to non-existent position_x elements. As written right now, nPunctures must be at most 3 (and currently it could go up to 10 given PunctureTracker's param.ccl, can it?).

Very good. I add assert(nPunctures <= 3) // BoxInBox currently hardcodes position[3] when we are using BoxInBox

@lwJi lwJi closed this Nov 21, 2025
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