JuliaLinearAlgebra / MatrixDepot.jl

An Extensible Test Matrix Collection for Julia
http://matrixdepotjl.readthedocs.org/
Other
75 stars 22 forks source link

Fix bug in frank matrix for non-integer types #46

Closed neil-lindquist closed 3 years ago

neil-lindquist commented 4 years ago

If the frank matrix is generated for a non-integer type, it errors. The error comes from trying to use p as an index when p is an array of non-integers.

KlausC commented 4 years ago

Thanks for the fix. You might add a test case for that.

codecov[bot] commented 4 years ago

Codecov Report

Merging #46 into master will increase coverage by 0.36%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   91.08%   91.45%   +0.36%     
==========================================
  Files          12       12              
  Lines        1873     1942      +69     
==========================================
+ Hits         1706     1776      +70     
+ Misses        167      166       -1
Impacted Files Coverage Δ
src/higham.jl 94.63% <ø> (+0.16%) :arrow_up:
src/MatrixDepot.jl 95.45% <0%> (-4.55%) :arrow_down:
src/matrixmarket.jl 91.4% <0%> (-1.53%) :arrow_down:
src/download.jl 93.46% <0%> (-0.93%) :arrow_down:
src/common.jl 96.81% <0%> (+0.02%) :arrow_up:
src/datareader.jl 88.31% <0%> (+0.15%) :arrow_up:
src/regu.jl 92.23% <0%> (+0.36%) :arrow_up:
src/markdown.jl 98.7% <0%> (+1.51%) :arrow_up:
src/types.jl 71.92% <0%> (+1.75%) :arrow_up:
... and 1 more

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 5b7010a...c6e1aee. Read the comment docs.

neil-lindquist commented 4 years ago

I added a test case for the case that errors. As a side note, I realized that my initial comment wasn't clear that the error only arises when using the reflected form of the matrix.

KlausC commented 4 years ago

bump @andreasnoack