-
Notifications
You must be signed in to change notification settings - Fork 8
Various fixes to KirlikSayin #108
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
Conversation
@@ -99,68 +100,62 @@ function optimize_multiobjective!(algorithm::KirlikSayin, model::Optimizer) | |||
variables = MOI.get(model.inner, MOI.ListOfVariableIndices()) | |||
n = MOI.output_dimension(model.f) | |||
yI, yN = zeros(n), zeros(n) | |||
δ = sense == MOI.MIN_SENSE ? -1 : 1 | |||
# This tolerance is really important! | |||
δ = 1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this tolerance arbitrary? Why 1.0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these algorithms target integer programs, the e-constraints enforces f_i(x) < u_i and the bound are integer as well.
return status, nothing | ||
end | ||
_, Y = _compute_point(model, variables, f_i) | ||
yN[i] = Y | ||
yN[i] = Y + δ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the +1
(now + δ
) was needed because we have a separate constraint that f_i <= u - δ
. So the box bounds need to account for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a +1 here because the \delta was for e-constraints. Having +1 makes it consistent with KS and DR. Original algorithm uses some big M.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #108 +/- ##
==========================================
- Coverage 98.78% 98.78% -0.01%
==========================================
Files 9 9
Lines 991 990 -1
==========================================
- Hits 979 978 -1
Misses 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Part of #105
Alternative to #107
I'll comment in-line.