RFC: Changes to allow JuMPeR to use matrix operations #513
Conversation
| @@ -244,7 +244,8 @@ setPrintHook(m::Model, f) = (m.printhook = f) | |||
| ############################################################################### | |||
| # Variable class | |||
| # Doesn't actually do much, just a pointer back to the model | |||
| immutable Variable <: ReverseDiffSparse.Placeholder | |||
| abstract AbstractVariable <: ReverseDiffSparse.Placeholder | |||
There was a problem hiding this comment.
This is to allow JuMPTypes to be more general. It may also allow for other code savings in JuMPeR, but just focussed on linalg right now
There was a problem hiding this comment.
I don't like the name though
|
I've been thinking about this for a few weeks now, and how about something like abstract AbstractScalar <: ReverseDiffSparse.Placeholder
immutable Variable <: AbstractScalar ... end
type GenericAffExpr <: AbstractScalar ... end
type JuMPeR.Uncertain <: AbstractScalar ... endThe only thing I'm unsure about is the |
|
Or maybe name it something else like |
| @@ -496,13 +496,13 @@ end | |||
|
|
|||
| # Don't do size checks here in _return_array, defer that to (*) | |||
| function _return_array{R,S}(A::AbstractArray{R}, x::AbstractArray{S,1}) | |||
| Q = (R <: JuMPTypes && S <: JuMPTypes) ? QuadExpr : AffExpr | |||
| Q = promote_type(R,S) | |||
There was a problem hiding this comment.
This works the same way on v0.3 and v0.4? If so that's great, I was never happy about this.
There was a problem hiding this comment.
Things are not working entirely, but are at least working partially
IainNZ
changed the title
|
LGTM. Does it make sense to make |
Changes to allow JuMPeR to use matrix operations
| (*)(lhs::AffExpr, rhs::Variable) = (*)(rhs,lhs) | ||
| (/)(lhs::AffExpr, rhs::Variable) = error("Cannot divide affine expression by a variable") | ||
| # AffExpr--Norm | ||
| (+){C,V}(lhs::GenericAffExpr{C,V}, rhs::GenericNorm{2,C,V}) = GenericSOCExpr{C,V}(copy(rhs), one(C), copy(lhs)) | ||
| (-){C,V}(lhs::GenericAffExpr{C,V}, rhs::GenericNorm{2,C,V}) = GenericSOCExpr{C,V}(copy(rhs), -one(C), copy(lhs)) | ||
| # AffExpr--AffExpr | ||
| (+){T<:GenericAffExpr}(lhs::T, rhs::T) = (operator_warn(lhs,rhs); GenericAffExpr(vcat(lhs.vars,rhs.vars),vcat(lhs.coeffs, rhs.coeffs),lhs.constant+rhs.constant)) | ||
| (-){T<:GenericAffExpr}(lhs::T, rhs::T) = GenericAffExpr(vcat(lhs.vars,rhs.vars),vcat(lhs.coeffs,-rhs.coeffs),lhs.constant-rhs.constant) | ||
| (+){C,V<:JuMPTypes}(lhs::GenericAffExpr{C,V}, rhs::GenericAffExpr{C,V}) = (operator_warn(lhs,rhs); GenericAffExpr(vcat(lhs.vars,rhs.vars),vcat(lhs.coeffs, rhs.coeffs),lhs.constant+rhs.constant)) |
There was a problem hiding this comment.
Why change this? The previous definition seems fine.
There was a problem hiding this comment.
Ambiguity warnings, again. Plus, new definition also seemed fine, and this was a minimal change that I couldn't see causing any issues that couldn't be fixed with the new abstract type
|
To be clear, the only things I think really needed to be done was the notion of |
_multiply_type, to determine the storage for matrix multiply