Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 2095 #2107

Merged
merged 3 commits into from
Jan 9, 2025
Merged

Fix 2095 #2107

merged 3 commits into from
Jan 9, 2025

Conversation

fwesselm
Copy link
Collaborator

@fwesselm fwesselm commented Jan 7, 2025

Fix #2095:

  • Pointers stored in HighsMatrixSlice may get invalidated if the coefficient matrix is reallocated (e.g. when non-zeros are added in HPresolve::addToMatrix).
  • The proposed fix is to directly iterate over the non-zero elements (positions in the matrix) instead of using HighsMatrixSlice when the matrix is modified while iterating.

Copy link
Member

@jajhall jajhall left a comment

Choose a reason for hiding this comment

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

Looks correct, and more robust than my fix. I guess you'll regression test it

Are there other places in presolve where such an issue could be exposed?

@fwesselm
Copy link
Collaborator Author

fwesselm commented Jan 8, 2025

Looks correct, and more robust than my fix. I guess you'll regression test it

Are there other places in presolve where such an issue could be exposed?

Regression testing is ongoing.

I looked for calls to HPresolve::addToMatrix while iterating over a HighsMatrixSlice in presolve, and I think the two occurrences in this PR are the only problematic ones. I hope I did not overlook something.

@fwesselm
Copy link
Collaborator Author

fwesselm commented Jan 9, 2025

Testing shows that, while HiGHS behavior is affected, the overall results look reasonable.

@jajhall jajhall merged commit b6a528e into ERGO-Code:latest Jan 9, 2025
114 of 115 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.

2 participants