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: Optimization-based bound tightening utility #103

Merged
merged 25 commits into from
Aug 21, 2020
Merged

ADD: Optimization-based bound tightening utility #103

merged 25 commits into from
Aug 21, 2020

Conversation

tasseff
Copy link
Member

@tasseff tasseff commented Aug 21, 2020

This adds a draft version of the optimization-based bound tightening utility. It also includes various bug fixes encountered during implementation of this utility.

Copy link
Member

@ccoffrin ccoffrin left a comment

Choose a reason for hiding this comment

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

I only did a quick review but I will make the following suggestion.

If you can avoid having the min-width and precision differ based on the variable types, I think that would be greatly preferable. This will add quite a few parameters that a user might need to tune. Also note that the smallest possible values for these parameters that are still correct will vary based on optimality tolerance of the solver used for bound tightening; so if the user can specify that solver, they should also be able to provide a globally correct min-width and precision.

I'll also note that specifying the precision as prec::Int is convenient from an implementation standpoint, but I think it is less intuitive to a would be user. I would make this a 1e-x type of tolerance, which is a fairly standard approach for specifying tolerances.

.travis.yml Outdated
@@ -4,15 +4,15 @@ os:
- osx
julia:
- 1.0
- 1.4
- 1.5
Copy link
Member

Choose a reason for hiding this comment

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

I recently learned we can use 1 to sick to the latest version of 1.x

@ccoffrin
Copy link
Member

I'll also suggest adding a note to the change log. :-)

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #103 into master will decrease coverage by 0.05%.
The diff coverage is 86.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   84.78%   84.72%   -0.06%     
==========================================
  Files          23       24       +1     
  Lines        1840     2108     +268     
==========================================
+ Hits         1560     1786     +226     
- Misses        280      322      +42     
Impacted Files Coverage Δ
src/WaterModels.jl 100.00% <ø> (ø)
src/core/constraint.jl 100.00% <ø> (ø)
src/core/data.jl 43.35% <0.00%> (-1.90%) ⬇️
src/core/objective.jl 100.00% <ø> (+22.22%) ⬆️
src/form/directed_flow.jl 91.16% <37.50%> (-6.88%) ⬇️
src/io/epanet.jl 71.95% <55.55%> (-0.15%) ⬇️
src/form/undirected_flow.jl 97.40% <71.42%> (-2.60%) ⬇️
src/util/obbt.jl 90.90% <90.90%> (ø)
src/prob/wf.jl 98.82% <91.66%> (-1.18%) ⬇️
src/core/constraint_template.jl 94.30% <94.87%> (-0.48%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6d734d...cb38a01. Read the comment docs.

@tasseff tasseff merged commit 6136474 into master Aug 21, 2020
@tasseff tasseff deleted the obbt branch August 21, 2020 20:54
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