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

Heuristics Interface #162

Merged
merged 34 commits into from
Jan 15, 2024
Merged

Heuristics Interface #162

merged 34 commits into from
Jan 15, 2024

Conversation

dhendryc
Copy link
Collaborator

Interface for our own predefined and user custom heuristics.

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (ee5cc6c) 86.23% compared to head (1b554c9) 87.22%.
Report is 12 commits behind head on main.

Files Patch % Lines
src/polytope_blmos.jl 91.52% 5 Missing ⚠️
src/MOI_bounded_oracle.jl 87.50% 4 Missing ⚠️
src/callbacks.jl 50.00% 4 Missing ⚠️
src/heuristics.jl 96.15% 3 Missing ⚠️
src/node.jl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #162      +/-   ##
==========================================
+ Coverage   86.23%   87.22%   +0.98%     
==========================================
  Files          17       18       +1     
  Lines        1482     1620     +138     
==========================================
+ Hits         1278     1413     +135     
- Misses        204      207       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dhendryc
Copy link
Collaborator Author

We roll the dice on every heuristics, i.e. we can run more than one heuristics at a node.

@dhendryc
Copy link
Collaborator Author

We should also think about whether the heuristics should be called at every node or if this should depend on the depth in the tree etc.

@dhendryc dhendryc marked this pull request as ready for review November 15, 2023 17:35
src/heuristics.jl Outdated Show resolved Hide resolved
src/heuristics.jl Outdated Show resolved Hide resolved
src/heuristics.jl Outdated Show resolved Hide resolved
src/interface.jl Outdated Show resolved Hide resolved
src/polytope_blmos.jl Outdated Show resolved Hide resolved
test/LMO_test.jl Outdated Show resolved Hide resolved
@pokutta
Copy link
Member

pokutta commented Nov 15, 2023

just went over this -> it would be good if a heuristics could return more than one solution as they often create multiple sols. maybe they can return a list or something instead of a single solution.

@dhendryc
Copy link
Collaborator Author

Count the calls to the LMO in the heuristcs:

  1. Wrap the inner LMO in a TrackingLMO just for the heuristics:
lmo = tracking_lmo.inner
heuristic_lmo = TrackingLMO()
run_heuristic(heuristic_lmo)
collect_statistics(heuristic_lmo)

and have a field specifically for the heuristic LMO calls in the TrackingLMO.

@pokutta
Copy link
Member

pokutta commented Nov 19, 2023

  • bumped to prob 1.0 for standard rounding
  • added two simple heuristics -> not perfect yet but a start
  • adjusted two examples to use the heuristics (nonlinear.jl and portfolio.jl)
  • cleaned up some imports

src/heuristics.jl Outdated Show resolved Hide resolved
@dhendryc
Copy link
Collaborator Author

dhendryc commented Nov 20, 2023

The feasibility check after the rounding can become expensive. It would be good to enable the user to set the probability of the rounding to something smaller than 1.0 in case the feasibility check of their LMO is expensive.

@matbesancon
Copy link
Member

More generically, we can define a property for heuristics, that describes whether they are feasible by design or require a feasibility check, and then a boolean keyword that allows heuristics or not

@pokutta
Copy link
Member

pokutta commented Nov 20, 2023

The feasibility check after the rounding can become expensive. It would be good to enable the user to set the probability of the rounding to something smaller than 1.0 in case the feasibility check of their LMO is expensive.

you mean of the "standard" rounding -> yes good point. how did you do it before making it a heuristic?

@pokutta
Copy link
Member

pokutta commented Nov 20, 2023

More generically, we can define a property for heuristics, that describes whether they are feasible by design or require a feasibility check, and then a boolean keyword that allows heuristics or not

we have the flag that we return whether feasiblity needs to be checked or not. i set them to zero for those that are guaranteed to be feasible. do we need more?

@dhendryc
Copy link
Collaborator Author

More generically, we can define a property for heuristics, that describes whether they are feasible by design or require a feasibility check, and then a boolean keyword that allows heuristics or not

we have the flag that we return whether feasiblity needs to be checked or not. i set them to zero for those that are guaranteed to be feasible. do we need more?

No, that's fine. Then we only need a property for the "standard" rounding.
And yes, before we rounded every time. I think we didn't have large enough problems yet where the feasibility test was problematic. This showed up for the network design problems with Kartikey.

@dhendryc dhendryc merged commit 4742469 into main Jan 15, 2024
5 checks passed
@dhendryc dhendryc deleted the heuristics branch January 30, 2024 13:25
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.

3 participants