Skip to content

Conversation

odow
Copy link
Member

@odow odow commented Nov 24, 2021

This reverts commit 1a3c637.

This caused widespread failures in SolverTests for solvers using a caching optimizer.
#1656 (comment)

@blegat
Copy link
Member

blegat commented Nov 24, 2021

When you have a CachingOptimizer in EMPTY_OPTIMIZER state, I think the behavior of setting an attribute unsupported by the optimize or copy_toing a model with an attribute unsupported by the optimizer should be the same. With this PR, it will throw on the copy but not on set. Adding the check in

if m.state == ATTACHED_OPTIMIZER
would throw it in both cases.
Note that if we do it for the attributes, we should do it for add_constraint too.

Another (simpler IMO) solution would be to modify the test into something like

@test_throws ... begin
    copy_to(...)
    optimize!(...)
end

@odow odow mentioned this pull request Nov 24, 2021
20 tasks
@odow
Copy link
Member Author

odow commented Nov 24, 2021

Yeah okay. Maybe it's a bad test. We don't have a lot of consistency around when errors are thrown.

@odow
Copy link
Member Author

odow commented Nov 25, 2021

Closing in favor of #1686

@odow odow closed this Nov 25, 2021
@odow odow deleted the od/revert-1656 branch November 25, 2021 01:03
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