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

Switch the default options for EpsilonConstraint #31

Merged
merged 3 commits into from
Feb 12, 2023

Conversation

odow
Copy link
Member

@odow odow commented Feb 12, 2023

@xgandibleux is this better?

It'd mean in your JuMP examples you could just use

set_optimizer_attribute(model, MOA.Algorithm(), MOA.EpsilonConstraint())

instead of

set_optimizer_attribute(model, MOA.Algorithm(), MOA.EpsilonConstraint())
set_optimizer_attribute(model, MOA.ObjectiveAbsoluteTolerance(1), 1)

@codecov
Copy link

codecov bot commented Feb 12, 2023

Codecov Report

Base: 93.30% // Head: 94.18% // Increases project coverage by +0.87% 🎉

Coverage data is based on head (db9f5d4) compared to base (5575499).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   93.30%   94.18%   +0.87%     
==========================================
  Files           6        6              
  Lines         508      516       +8     
==========================================
+ Hits          474      486      +12     
+ Misses         34       30       -4     
Impacted Files Coverage Δ
src/MultiObjectiveAlgorithms.jl 90.00% <100.00%> (+0.67%) ⬆️
src/algorithms/EpsilonConstraint.jl 96.72% <100.00%> (+5.81%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@xgandibleux
Copy link

Indeed. The step value of 1 is usual for $\epsilon$. Thus, setting it as default value is fine, and letting to the user the possibility to setup the algorithm with a non-negative float value via the parameter (preferably named e.g. "step", "stepForEpsilon" than "tolerance" which evokes possible precision problems, or approximate solutions).

@odow
Copy link
Member Author

odow commented Feb 12, 2023

or approximate solutions

But isn't this exactly what is happening? If I set the ObjectiveAbsoluteTolerance to 2.0 in an integer problem, then I might miss solutions. (The complication being that they can be out by at most that tolerance / 2.)

There's definitely a trade-off. We could either introduce lots of algorithm-specific options, or try to re-use common names, even if they're slightly clunky.

Would EpsilonConstraintStep() be a better option?

@xgandibleux
Copy link

EpsilonConstraintStep()is perfect.

@xgandibleux
Copy link

I was not fan of tolerance since it evokes a parameter to deal with numerical approximations (resulting e.g. from the MIP resolution).

But you are right. With $\epsilon$=2 the algorithm may miss non dominated points (and we encounter some uses of this algorithm where the value of $\epsilon$ is dynamically tuned, e.g. when a timeout limit is taking into account, which is not into in the version available here, and somewhere not relevant to be coded I think because specific to a problem and outputs expected).

@odow odow force-pushed the od/default-epsilon-constraint branch from ce3910f to 954fdb7 Compare February 12, 2023 19:59
@odow odow force-pushed the od/default-epsilon-constraint branch from 954fdb7 to 95ca9c4 Compare February 12, 2023 20:07
@odow odow merged commit 1f5ef85 into master Feb 12, 2023
@odow odow deleted the od/default-epsilon-constraint branch February 12, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants