Skip to content

Conversation

@odow
Copy link
Member

@odow odow commented Dec 6, 2020

@codecov
Copy link

codecov bot commented Dec 6, 2020

Codecov Report

Merging #2397 (5a41da8) into master (52020e3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2397   +/-   ##
=======================================
  Coverage   91.61%   91.62%           
=======================================
  Files          42       42           
  Lines        4451     4452    +1     
=======================================
+ Hits         4078     4079    +1     
  Misses        373      373           
Impacted Files Coverage Δ
src/callbacks.jl 100.00% <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 52020e3...5a41da8. Read the comment docs.

# see if optimize! has been called. Solutions are:
# 1) defining is_set_by_optimize = false
# 2) adding a flag to JuMP to store whether it is in a callback
# 3) adding IN_OPTIMIZE to termination_status for callbacks
Copy link
Member

Choose a reason for hiding this comment

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

I would vote for 3) as 1) would not work as the CachingOptimizer will try to query from the cache.
In addition, we could have is_set_in_optimize and the CachingOptimizer checks is_set_in_optimizer || is_set_by_optimizer.

Copy link
Member Author

Choose a reason for hiding this comment

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

It might actually be easier just to have this slight work-around for callback attributes. We don't have many, and it's simpler than defining a new check with all the correct plumbing. If we have many more attributes, we can revisit this.

@odow odow merged commit 1e6b5d8 into master Dec 10, 2020
@odow odow deleted the od/cb_node_status branch December 10, 2020 22:59
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.

Add support for CallbackNodeStatus

3 participants