Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release that supports Julia 0.6 #14

Merged
merged 7 commits into from
Jun 2, 2017
Merged

Release that supports Julia 0.6 #14

merged 7 commits into from
Jun 2, 2017

Conversation

abhijithanilkumar
Copy link
Contributor

@abhijithanilkumar abhijithanilkumar commented May 21, 2017

  • Use StaticArrays instead of FixedSizeArrays
  • Fix Deprecation warnings
  • Fix type issues

test/runtests.jl Outdated
@@ -151,8 +151,8 @@ jagmesh_adj = jagmesh()
for i in 1:size(a,1)
p = Int32[]
for e in collect(edges(g))
if e[1] == i
push!(p,e[2])
if e.src == i
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be src(e) for LightGraphs - use the accessor function, not the internal field.

Same with L155 - dst(e).

@codecov
Copy link

codecov bot commented May 21, 2017

Codecov Report

Merging #14 into master will increase coverage by 0.3%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #14     +/-   ##
=========================================
+ Coverage   97.22%   97.53%   +0.3%     
=========================================
  Files           7        7             
  Lines         324      324             
=========================================
+ Hits          315      316      +1     
+ Misses          9        8      -1
Impacted Files Coverage Δ
src/stress.jl 98.38% <ø> (ø) ⬆️
src/spectral.jl 100% <100%> (ø) ⬆️
src/shell.jl 100% <100%> (ø) ⬆️
src/spring.jl 97.5% <0%> (+2.5%) ⬆️

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 ce5f15f...7474d77. Read the comment docs.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e563502 on v0.6-dev into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e47a426 on v0.6-dev into ** on master**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e47a426 on v0.6-dev into ** on master**.

src/stress.jl Outdated
map(D) do d
x = T(d^(-2.0))
isfinite(x) ? x : zero(T)
x = Float64(d^(-2.0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would you want to restrict it only to Float64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When generalizing, in 0.6 it's creating a sparse matrix of type {Any,Int64}. In 0.5 it was of type {T,Int64}. So in 0.6 this is throwing an error while calling zeros() function later in weightedlaplacian. Also, isn't it safe to assume that the output of d^-2.0 is always floating point?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, isn't it safe to assume that the output of d^-2.0 is always floating point?

Float64 is not the only float type in julia...

And Any as the matrix eltype shouldn't happen, so you're just working around a bug that must have happened earlier!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the eltype, map function is generating different outputs in Julia 0.5 and 0.6
In 0.5, it generates sparse matrix with Float elements :

julia> function fx(a,t=eltype(a))
              map(a) do d
              x=t(d^(-2.0))
              isfinite(x)?x:zero(t)
              end
              end
fx (generic function with 2 methods)

julia> fx(a,Float32)
30×30 sparse matrix with 116 Float32 nonzero entries:
	[2 ,  1]  =  1.0
	[3 ,  1]  =  1.0
	[4 ,  1]  =  1.0
	[5 ,  1]  =  1.0
	[6 ,  1]  =  1.0
	[7 ,  1]  =  1.0
	[8 ,  1]  =  1.0

In 0.6 it is generating matrix with Any elements :

julia> function fx(a,t=eltype(a))
              map(a) do d
              x=t(d^(-2.0))
              isfinite(x)?x:zero(t)
              end
              end
fx (generic function with 2 methods)

julia> fx(a,Float32)
30×30 SparseMatrixCSC{Any,Int64} with 116 stored entries:
  [2 ,  1]  =  1.0
  [3 ,  1]  =  1.0
  [4 ,  1]  =  1.0
  [5 ,  1]  =  1.0
  [6 ,  1]  =  1.0
  [7 ,  1]  =  1.0
  [8 ,  1]  =  1.0

What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must be d^t(-2.0) then...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried the above suggestions and still no luck :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I have absolutely no time to care about this package right now.
If you can't figure it out, I guess that's then just how it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your time :) I'll try to find a way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've figured out how to do this:

julia> function fx(a,t=eltype(a))::SparseMatrixCSC{t, Int64}
                     map(a) do d
                     x=t(d^(-2.0))
                     isfinite(x)?x:zero(t)
                     end
                     end
fx (generic function with 2 methods)

julia> g = Graph(3,3)
{3, 3} undirected simple Int64 graph

julia> fx(adjacency_matrix(g), Float32)
3×3 SparseMatrixCSC{Float32,Int64} with 6 stored entries:
  [2, 1]  =  1.0
  [3, 1]  =  1.0
  [1, 2]  =  1.0
  [3, 2]  =  1.0
  [1, 3]  =  1.0
  [2, 3]  =  1.0

Return type annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion! That fixes the issue.

@coveralls
Copy link

coveralls commented May 31, 2017

Coverage Status

Changes Unknown when pulling 7474d77 on v0.6-dev into ** on master**.

@abhijithanilkumar abhijithanilkumar merged commit 094e4b5 into master Jun 2, 2017
@abhijithanilkumar abhijithanilkumar deleted the v0.6-dev branch June 2, 2017 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants