matbesancon / MathOptSetDistances.jl

Distances to sets for MathOptInterface
MIT License
24 stars 4 forks source link

add projection methods #5

Closed akshay326 closed 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #5 into master will increase coverage by 8.25%. The diff coverage is 86.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #5      +/-   ##
==========================================
+ Coverage   67.18%   75.44%   +8.25%     
==========================================
  Files           3        4       +1     
  Lines         128      224      +96     
==========================================
+ Hits           86      169      +83     
- Misses         42       55      +13     
Impacted Files Coverage Δ
src/MathOptSetDistances.jl 100.00% <ø> (ø)
src/distances.jl 50.00% <0.00%> (-25.00%) :arrow_down:
src/sets.jl 66.66% <ø> (ø)
src/projections.jl 88.29% <88.29%> (ø)

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 c74591c...3adbab3. Read the comment docs.

matbesancon commented 4 years ago

The general rule for which distance to implement: whenever you can

  1. implement one of the specific distances
  2. make the DefaultDistance fall back to the most "obvious" one
matbesancon commented 4 years ago

Fixes #4

matbesancon commented 4 years ago

A good property to test for all sets: The distance_to_set of the projection should always be 0

matbesancon commented 4 years ago

the docstring changes can be applied everywhere (in all docstrings)

matbesancon commented 4 years ago

Overall, I am not sure the dual argument is necessary, if people need the projection on the dual, they can just project(dist, z, dual(s))

matbesancon commented 4 years ago

this will also simplify some functions IMO

matbesancon commented 4 years ago

bump :)

matbesancon commented 4 years ago

See codecov comments for where to add tests

akshay326 commented 4 years ago

add test for projections and gradients

akshay326 commented 4 years ago

actually this was useful: found and fixed a bug in one of the projection methods