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

feat: use ilpy.expressions in motile Constraints #30

Merged
merged 20 commits into from
May 12, 2023

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Mar 17, 2023

This updates most of the motile constraints and costs to use ilpy.Expression introduced in funkelab/ilpy#9 and released in ilpy v0.3.0

Another notable (API breaking) change is that indexing into a Variable (i.e. Variable.__get_item__) now returns an ilpy.Variable instance, rather than an integer, as per #30 (comment). In many cases, that Variable can be used exactly as the integer could before (it can be cast to an int with int(), it can be used as an index in a Solution, etc...), but it can also be used to create expressions.

@funkey
Copy link
Member

funkey commented Mar 24, 2023

I wonder if we shouldn't make the return value of variables[key] an ilpy.Variable right away instead of returning the index?

ilpy.Variables have an index, and that is precisely the one we need. This would make writing constraints a bit easier, but most importantly

for node, selection_indicator in selection_indicators.items():
    # use selection_indicator here

assumes less familiarity with the internals than

for node, index in selection_indicators.items():
    # know what to do with an index

@codecov-commenter
Copy link

codecov-commenter commented May 3, 2023

Codecov Report

Merging #30 (db64ba6) into main (624d903) will decrease coverage by 0.30%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
- Coverage   92.77%   92.47%   -0.30%     
==========================================
  Files          30       30              
  Lines         747      718      -29     
==========================================
- Hits          693      664      -29     
  Misses         54       54              
Impacted Files Coverage Δ
motile/constraints/constraint.py 100.00% <100.00%> (ø)
motile/constraints/max_children.py 100.00% <100.00%> (ø)
motile/constraints/max_parents.py 100.00% <100.00%> (ø)
motile/constraints/select_edge_nodes.py 100.00% <100.00%> (ø)
motile/costs/features.py 82.85% <100.00%> (+1.03%) ⬆️
motile/solver.py 96.96% <100.00%> (ø)
motile/variables/node_appear.py 100.00% <100.00%> (ø)
motile/variables/variable.py 63.88% <100.00%> (+1.03%) ⬆️

@tlambert03 tlambert03 changed the title WIP: use ilpy.expressions feat: use ilpy.expressions in motile Constraints May 6, 2023
@tlambert03
Copy link
Member Author

This is now ready for review @funkey

@funkey
Copy link
Member

funkey commented May 12, 2023

Perfect, looks all good. Thanks a lot!

@funkey funkey merged commit 393ead4 into funkelab:main May 12, 2023
10 checks passed
@tlambert03 tlambert03 deleted the expr branch May 12, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants