Skip to content

Conversation

odow
Copy link
Member

@odow odow commented Jul 15, 2021

This started off by adding some tests to box.jl, but I got confused. There's a muddle between box.jl and model.jl, and who owns what and does what isn't clear. Making this commit now, and then I'm going to try and encapsulate all of the mask stuff into box.jl.

Part of #1457

@odow odow added the Submodule: Utilities About the Utilities submodule label Jul 15, 2021
@blegat
Copy link
Member

blegat commented Jul 15, 2021

Yes, it's a bit sketchy. The issue is that it's quite performance critical for MOIU.Model as we built this data-structure so to fix the performance regression from JuMP v0.18 to v0.19 with SingleVariable constraint

@odow
Copy link
Member Author

odow commented Jul 15, 2021

I had a WIP that's coming along nicely. Just finishing up the tests.

@odow odow changed the title WIP: refactoring of Box.jl Refactor box.jl Jul 15, 2021
@odow odow requested a review from blegat July 15, 2021 04:38
@blegat blegat mentioned this pull request Jul 15, 2021
@odow odow requested a review from blegat July 16, 2021 05:18
@odow odow requested a review from blegat July 21, 2021 02:29
@blegat
Copy link
Member

blegat commented Jul 21, 2021

I like how simpler AbstractModel is

@odow odow merged commit 1f76df0 into master Jul 21, 2021
@odow odow deleted the od/box branch July 21, 2021 22:19
@odow
Copy link
Member Author

odow commented Jul 21, 2021

Yeah, less state to carry around, easier to test, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Utilities About the Utilities submodule
Development

Successfully merging this pull request may close these issues.

2 participants