henselman-petrusek / Eirene.jl

Julia library for homological persistence
Other
117 stars 21 forks source link

julia 1.1 can't do subtractions of type "number - matrix" #5

Closed pegger0709 closed 5 years ago

pegger0709 commented 5 years ago

Hello, I think I found a very minor bug in the barcode function. Using julia 1.1, I ran eirene on a symmetric distance matrix without problems, but could not compute the Betti curve.

julia> C = eirene(distance_matrix);
julia> B = betticurve(C, dim=1);
ERROR: MethodError: no method matching -(::Int64, ::Array{Int64,2})
Closest candidates are:
  -(::Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}) at int.jl:51
  -(::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}, ::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}) where T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8} at int.jl:52
  -(::Union{Int16, Int32, Int64, Int8}, ::BigInt) at gmp.jl:449
  ...
Stacktrace:
 [1] #barcode#58(::Int64, ::Bool, ::Function, ::Dict{String,Any}) at /home/pegger/.julia/packages/Eirene/l24p9/src/Eirene.jl:5103
 [2] #barcode at ./none:0 [inlined]
 [3] #getbetticurve#42(::Bool, ::Function, ::Dict{String,Any}, ::Int64) at /home/pegger/.julia/packages/Eirene/l24p9/src/Eirene.jl:4539
 [4] #betticurve#63 at ./none:0 [inlined]
 [5] (::getfield(Eirene, Symbol("#kw##betticurve")))(::NamedTuple{(:dim,),Tuple{Int64}}, ::typeof(betticurve), ::Dict{String,Any}) at ./none:0
 [6] top-level scope at none:0

I looked at the barcode function and I think that the issue could be solved if line 5103 instead if reading bc = length(D["ocg2rad"])-bc would read bc = length(D["ocg2rad"])*ones(size(bc))-bc Hope this helps! Philip Egger

pegger0709 commented 5 years ago

On second thought, perhaps it would be even easier to simply replace the operator - by the operator .- ?

lachrimae commented 5 years ago

Thanks a lot, Philip. I am going to fork with the edit you've suggested so I can run this on my machine.

I'd like to note that this problem affects more than just the betti curve computation. It's prohibiting me from getting the class representatives for a non-Euclidean distance-matrix dataset. I'm a bit of a GitHub noob so I'm not sure what the protocol is for getting an edit like this pushed to the original repository. I'll probably leave that up to you and Eetion.

henselman-petrusek commented 5 years ago

Thanks for the comments + pull request! Changes from lachrimae are merged.

Haven't managed to reproduce the error for class reps of the noneuclidean distance matrix. If it's possible to share the code that produces this error (perhaps with a smaller input matrix), that would be very helpful. Thanks!

lachrimae commented 5 years ago

Hi Eetion, if I recall correctly, the traceback I was getting for the class reps issue was leading to the same part of the source code as the issue with betti curve computation. I will retry things with the old code and confirm that this is the case. There's a chance I just didn't understand the API at that point.

henselman-petrusek commented 5 years ago

Hi Lachrimae, thanks again for your input on this. I'm going to close the issue for now, but if it surfaces again, feel free to re-open. Thanks.