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

added linear Oettli-Präger solver #44

Merged
merged 8 commits into from
Jul 14, 2021
Merged

added linear Oettli-Präger solver #44

merged 8 commits into from
Jul 14, 2021

Conversation

lucaferranti
Copy link
Member

This algorithm converts the nonlinear system of real inequalities obtained with Oettli-Präger theorem to 2^n systems of linear real inequalities (one for each orthant) which are then solved using LazySets.jl

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2021

Codecov Report

Merging #44 (24d3fcc) into main (de8f3f6) will decrease coverage by 4.60%.
The diff coverage is 60.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
- Coverage   95.98%   91.37%   -4.61%     
==========================================
  Files           8        9       +1     
  Lines         224      255      +31     
==========================================
+ Hits          215      233      +18     
- Misses          9       22      +13     
Impacted Files Coverage Δ
src/utils.jl 68.29% <0.00%> (-31.71%) ⬇️
src/IntervalLinearAlgebra.jl 100.00% <100.00%> (ø)
src/solvers/oettli.jl 100.00% <100.00%> (ø)
src/solvers/oettli_linear.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de8f3f6...24d3fcc. Read the comment docs.

@lucaferranti
Copy link
Member Author

would it be better to return an array of polyhedra or one UnionSet?

@lucaferranti lucaferranti requested review from dpsanders and mforets July 5, 2021 07:19
@lucaferranti lucaferranti marked this pull request as draft July 6, 2021 16:24
@lucaferranti
Copy link
Member Author

lucaferranti commented Jul 6, 2021

  • filter empty polytopes
  • fix plotting with array
  • check DiagDirections instead of list_orthants

@mforets
Copy link
Collaborator

mforets commented Jul 6, 2021

check DiagDirections instead of list_orthants

be sure to be using LazySets#v1.41.2

@lucaferranti lucaferranti marked this pull request as ready for review July 13, 2021 12:55
@lucaferranti
Copy link
Member Author

be sure to be using LazySets#v1.41.2

BoxDiagonalDirections works also in 1.47.2 but it returns also the "extra" vertical directions

@mforets
Copy link
Collaborator

mforets commented Jul 13, 2021

@lucaferranti
Copy link
Member Author

lucaferranti commented Jul 13, 2021

but that requires LazySets.jl#master? not it shouldn't

@mforets
Copy link
Collaborator

mforets commented Jul 13, 2021

no, it was added in release v1.47.2

@lucaferranti lucaferranti requested review from dpsanders and mforets and removed request for dpsanders and mforets July 13, 2021 13:22
@@ -9,8 +9,9 @@ function (opl::LinearOettliPrager)(A, b)
Ar = IntervalArithmetic.radius.(A)
br = IntervalArithmetic.radius.(b)

orthants = list_orthants(length(b))
polytopes = Vector{HPolytope}(undef, length(orthants))
polytopes = Vector{HPolytope}(undef, 2^n)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could give unexpected errors. try 2^50 and Julia gives ERROR: OutOfMemoryError(). try 2^100 and it returns an empty array, since it overflows.

should we print an error message if n is too big to handle with this method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it could be good I guess. An improved version of this solver could allow the user to cherry pick the orthants they want to go through (e.g. some quantities are constrained to be positive, have computed an enclosure and crossed out some orthants). I wonder what would be a user friendly interface to index the orthants

Copy link
Collaborator

@mforets mforets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor comments.

[1.0, -1.0]
```
"""
function list_orthants(n)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is no longer used; it could be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may still be useful to have a function that goes through all orthants here, especially since lazy sets is an optional dependency. But there's definitely room for improvement in the current version

@lucaferranti lucaferranti merged commit aa3dc59 into main Jul 14, 2021
@lucaferranti lucaferranti deleted the lf/oettli_linear branch July 14, 2021 16:10
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