Gradients2 #44
Gradients2 #44
Conversation
Codecov Report
@@ Coverage Diff @@
## master #44 +/- ##
==========================================
+ Coverage 91.99% 92.74% +0.74%
==========================================
Files 63 64 +1
Lines 1861 1901 +40
==========================================
+ Hits 1712 1763 +51
+ Misses 149 138 -11
Continue to review full report at Codecov.
|
| @@ -28,7 +31,6 @@ function prox!(y::AbstractArray{RC}, f::Linear{RC, A}, x::AbstractArray{RC}, gam | |||
| end | |||
There was a problem hiding this comment.
Not changed in PR, but should be
y .= x .- gamma.*(f.c)
for efficiency.
| @@ -75,3 +75,9 @@ Maximum | |||
| Quadratic | |||
| SumPositive | |||
| ``` | |||
|
|
|||
| ## Distances | |||
There was a problem hiding this comment.
This I have also put in the section describing calculus rules, since these are really modifiers of some other (indicator) functions; but I guess they can stay in both places.
There was a problem hiding this comment.
They were in the FUNCTIONS.md in the root, so I copied them here. But you are right, we should think of how we can unify the different documentations.
| @@ -27,7 +27,7 @@ is_convex(f::IndSOC) = true | |||
| is_set(f::IndSOC) = true | |||
|
|
|||
| function prox!{T <: Real}(y::AbstractArray{T,1}, f::IndSOC, x::AbstractArray{T,1}, gamma::T=one(T)) | |||
| nx = norm(x[2:end]) | |||
| @views nx = norm(x[2:end]) | |||
| @@ -17,6 +17,9 @@ end | |||
|
|
|||
| Linear(c::A) where {R <: RealOrComplex, A <: AbstractArray{R}} = Linear{R, A}(c) | |||
|
|
|||
| is_separable(f::Linear) = true | |||
| is_convex(f::Linear) = true | |||
There was a problem hiding this comment.
I would also add is_smooth = true if not already there.
| @@ -19,6 +19,7 @@ If `iterative=true`, then `prox!` is evaluated approximately using an iterative | |||
|
|
|||
| abstract type Quadratic <: ProximableFunction end | |||
|
|
|||
| is_convex(f::Quadratic) = true | |||
There was a problem hiding this comment.
This I have not previously put because one should really check that Q is positive semidefinite...
There was a problem hiding this comment.
Any thoughts on this @mfalt ? Of course checking the eigenvalues is not viable, but maybe the user should be able to assert whether the function is convex or not?
There was a problem hiding this comment.
No ok, the docstring says we assume Q to be positive definite (should be semidefinite, to be corrected). We may want to change this in the near future, not all quadratics are convex!
There was a problem hiding this comment.
Yes, I added this because of the documentation, but you are right, maybe some keyword from the user, acknowledging if it is positive(semi)-definite would be good.
Implemented most of the gradients according to issue #43, test still missing.
Update: Tests added, still missing: NormL21, Maximum, NuclearNorm, but those can be added at a later date if needed.