michaelbynum commented 2 years ago

In some cases, it is possible to compute an upper bound on zhat that is strictly negative. In such cases, setting the upper bound on z to be equal to the upper bound on zhat is incorrect and produces an infeasible model. Even if zhat is strictly negative, z can still be 0.

fracek commented 2 years ago

Good catch, I fixed this in the new formulation branch but didn't backport it to the old one. The checks are failing because of a compatibility thing with Pyomo 6.2, what should we do? Merge this and then I'll fix it, or fix and then you rebase?

michaelbynum commented 2 years ago

I can just fix it in this PR?

codecov[bot] commented 2 years ago

Codecov Report

Merging #17 (27aba43) into main (a89dca7) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #17   +/-   ##
=======================================
  Coverage   91.10%   91.10%           
=======================================
  Files          15       15           
  Lines         854      854           
  Branches      147      159   +12     
=======================================
  Hits          778      778           
  Misses         51       51           
  Partials       25       25

Impacted Files	Coverage Δ
src/omlt/gbt/formulation.py	`88.05% <ø> (ø)`
src/omlt/neuralnet/relu.py	`91.39% <100.00%> (ø)`

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 a89dca7...27aba43. Read the comment docs.

jalving commented 2 years ago

I vote for merging this PR since all tests are passing. I think I am running into this issue on a simple example.

fracek commented 2 years ago

I will push a release shortly

cog-imperial / OMLT

The upper bound on z should always be at least 0 #17

Codecov Report