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

refactor: keep ilpy usage inside of Solver.solve [demo] #61

Closed
wants to merge 9 commits into from

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Nov 4, 2023

Here's a demo of what I was proposing in #60,
where the only usage of ilpy is now in the Solver.solve method itself... limiting direct dependence on the business logic of ilpy (things like std::vector<Constraint>) while retaining the performance where it really matters (in the solving)

(most of the lines here are just due to the copying of ilpy.Expression into this library)

👀 - note there is also breaking change here, where motile.variables.Variable is renamed to motile.variables.Variables (note the plural). So as not to conflict with a single variable in an expression, and to better represent that it is in fact a dict of multiple Variable instsance

@tlambert03 tlambert03 marked this pull request as draft November 4, 2023 14:41
Copy link

codecov bot commented Nov 4, 2023

Codecov Report

Merging #61 (4a7a6f6) into main (37b6067) will increase coverage by 0.78%.
The diff coverage is 97.08%.

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   92.65%   93.43%   +0.78%     
==========================================
  Files          32       33       +1     
  Lines         762      975     +213     
==========================================
+ Hits          706      911     +205     
- Misses         56       64       +8     
Files Coverage Δ
motile/constraints/constraint.py 100.00% <100.00%> (ø)
motile/constraints/expression.py 92.10% <100.00%> (-0.40%) ⬇️
motile/constraints/max_children.py 100.00% <100.00%> (ø)
motile/constraints/max_parents.py 100.00% <100.00%> (ø)
motile/costs/features.py 81.08% <100.00%> (ø)
motile/solver.py 97.16% <100.00%> (+0.20%) ⬆️
motile/ssvm.py 100.00% <ø> (ø)
motile/variables/__init__.py 100.00% <100.00%> (ø)
motile/variables/edge_selected.py 100.00% <100.00%> (ø)
motile/variables/node_appear.py 100.00% <100.00%> (ø)
... and 5 more

@tlambert03 tlambert03 marked this pull request as ready for review November 6, 2023 19:11
@tlambert03 tlambert03 closed this Dec 30, 2023
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.

1 participant