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

Add two utility functions to HighsDomain.cpp #1784

Merged
merged 19 commits into from
Jun 30, 2024

Conversation

fwesselm
Copy link
Contributor

I added two small utility functions to src/mip/HighsDomain.cpp to remove identical code blocks:

  1. computeDelta for computing change in bounds on constraint activities
  2. boundRange for computing the bound range.

I also fixed some typos along the way.

@jajhall jajhall self-requested a review June 1, 2024 12:45
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.

I have to trust that your rationalisation of the duplicated code is correct. Did you do a MIP benchmark run?

@fwesselm
Copy link
Contributor Author

fwesselm commented Jun 1, 2024

I only tested on a handful of instances so far. I hope to have the full results by the end of next week, if that's OK.

accept = true;
else
accept = false;
bound = static_cast<double>(floor(boundVal + mipsolver->mipdata_->feastol));
Copy link
Contributor Author

@fwesselm fwesselm Jun 18, 2024

Choose a reason for hiding this comment

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

Floor is now computed (using HighsCDouble) before casting back to double. This causes a behavior change on 7 out of 854 problems that I tested.

accept = true;
else
accept = false;
bound = static_cast<double>(ceil(boundVal - mipsolver->mipdata_->feastol));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ceiling is now computed (using HighsCDouble) before casting back to double. This causes a behavior change on 7 out of 854 problems that I tested.

@fwesselm
Copy link
Contributor Author

Test results show that HiGHS' behavior is affected on problems neos-1605075, neos-619167, neos-693347, noswot, ns1685374, pigeon-10 and rocI-4-11 (see comments above). Other than that performance is identical.

@jajhall jajhall merged commit d630089 into ERGO-Code:latest Jun 30, 2024
110 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