JuliaIntervals / IntervalLinearAlgebra.jl

Linear algebra done rigorously
MIT License
36 stars 9 forks source link

Avoid broadcast over Q in tests #128

Closed dkarrasch closed 1 year ago

dkarrasch commented 2 years ago

PR description

Shouldn't my proposal yield the same result? I'm proposing this since broadcast over a Q matrix that has rather expensive getindex is somewhat an "abuse" of the fact that AbstractQ <: AbstractMatrix, see https://github.com/JuliaLang/julia/pull/46196. This popped up in a nanosoldier run. I'm sure this is just a quick convenience usage here, in real-life code this would be horribly slow.

In case it matters if you interval-lify before the product, one could replace this line by

A = Symmetric(IA.Interval.(Matrix(Q)) * D * IA.Interval.(Matrix(Q')))
codecov-commenter commented 2 years ago

Codecov Report

Merging #128 (c304a0e) into main (82bfa81) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #128   +/-   ##
=======================================
  Coverage   97.19%   97.19%           
=======================================
  Files          17       17           
  Lines         642      642           
=======================================
  Hits          624      624           
  Misses         18       18           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

lucaferranti commented 2 years ago

Thank you for the PR! Yes, I think you need to intervallify before taking the product, otherwise you couldn't ensure that the starting D, Q are actually included in the corresponding interval version of A. (Now that I relook at the code, I think in principle it would be enough to convert D to matrix of intervals, but that's tangential to the issue).

lucaferranti commented 2 years ago

(now there seem to be some probably unrelated compatibility issues with ModelingToolKit, I'll try to have a look at those)

dkarrasch commented 1 year ago

Gentle bump. CI failures appear only on windows, independently from the Julia version used.

lucaferranti commented 1 year ago

yes you are right, the failures seem unrelated. I'm merging this and I'll address that in a separate PR.