Skip to content

Fix crash when initializing a WordAlignmentMatrix with one dimension - #16

Open
pmachapman wants to merge 1 commit into
masterfrom
fix_1d_matrix_crash
Open

Fix crash when initializing a WordAlignmentMatrix with one dimension#16
pmachapman wants to merge 1 commit into
masterfrom
fix_1d_matrix_crash

Conversation

@pmachapman

Copy link
Copy Markdown
Collaborator

This PR fixes a crash exposed by the e2e test for sillsdev/machine.py#336, where the target pre-translations were blank.

Interestingly, the crash only occurs on Linux and macOS: https://github.com/sillsdev/thot/actions/runs/31657210462

The training data that exposes the crash is at crash_data.zip, and can be run with a unit test along the lines of:

TEST(SymmetrizedAlignmentModelTest, crash)
{
  chdir("/home/peter/machine/docker-compose/builds/6a7be055db58e46ea2adfd2f/model");
  auto direct = make_shared<FastAlignModel>();
  direct->load("src_trg_invswm");
  auto inverse = make_shared<FastAlignModel>();
  inverse->load("src_trg_swm");

  auto* model = new SymmetrizedAlignmentModel(direct, inverse);
  model->setHeuristic(SymmetrizationHeuristic::GrowDiagFinalAnd);

  for (int i = 0; i < 74; ++i)
  {
    WordAlignmentMatrix waMatrix;
    LgProb prob = model->getTrainingAlignment(i, waMatrix);
    EXPECT_EQ((double)prob, (double)SMALL_LG_NUM);
  }
}

@pmachapman
pmachapman force-pushed the fix_1d_matrix_crash branch from 20e5823 to f482cb3 Compare August 13, 2026 01:36
matrix = new bool*[I];
bool* pool = new bool[(size_t)I * J]{false};
const size_t size = std::max<size_t>(1, static_cast<size_t>(I) * J);
bool* pool = new bool[size]{false};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using an empty initializer is valid and gives the correct results, i.e.

    if (I > 0)
    {
      matrix = new bool*[I];
      bool* pool = new bool[(size_t)I * J]{};
      for (unsigned int i = 0; i < I; ++i, pool += J)
        matrix[i] = pool;
    }


bool WordAlignmentMatrix::getValue(unsigned int i, unsigned int j) const
{
return matrix[i][j];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need proper bounds checking here:

  if (i < I && j < J)
    return matrix[i][j];
  return false;

{
for (j = 0; j < aligVec.size(); ++j)
{
if (aligVec[j] > 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need proper bounds checking here:

      if (aligVec[j] > 0 && aligVec[j] <= I)

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.

2 participants