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

Make type of cell size subtype of AbstractFloat #9

Closed
wants to merge 1 commit into from

Conversation

OmriTreidel
Copy link
Contributor

Type of dx is too restrictive

@coveralls
Copy link

coveralls commented Oct 6, 2016

Coverage Status

Coverage decreased (-0.7%) to 94.521% when pulling 53d9e58 on fix-type-dx into 90dcaa2 on master.

@JoshChristie
Copy link
Contributor

Looks good. Not sure what's happening with failing on 0.6 and not liking list comprehensions.

@andyferris
Copy link

The error in question is:

generated function body is not pure. this likely means it contains a closure or comprehension.

Oh dear. This is interesting. The definition of "pure" is that a function has no side effects: it takes an input and returns an output and doesn't modify anything. Like a mathematical function.

In Julia we have the interesting situation that compiling things may have external consequences, such as creating new types. A closure is a kind of function-within-a-function that, as an internal Julia compiler implementation detail, creates a new Julia type that contains the captured variables. Thus, the side-effect is that calling the function generator creates a new type that in theory is visible after the compilation is complete (in practice seeing this is almost impossible for end users, but to the compiler guys it is important that things aren't changing underneath them during certain stages of compilation). Also note that this is exactly the same as issue JuliaLang/julia#16806

(A comprehension also does something funky at the compiler level - they are specially blessed things and probably do something like the closure does for performance reasons).

All this is to say that this is a problem that will probably never go away - we should plan to change the relevant code at some point. Probably isn't too hard.

@c42f
Copy link
Contributor

c42f commented Oct 7, 2016

Uh, what? I don't understand this PR. Aren't the following exactly equivalent:

foo(x::AbstractFloat) = x
bar{T<:AbstractFloat}(x::T) = x

I mean, with those definitions we have

julia> foo(1.0)
1.0

julia> bar(1.0)
1.0

julia> foo(big(1.0))
1.000000000000000000000000000000000000000000000000000000000000000000000000000000

julia> bar(big(1.0))
1.000000000000000000000000000000000000000000000000000000000000000000000000000000

@JoshChristie
Copy link
Contributor

Closing this. spatial_grid.jl was moved into SpatialGrids.jl

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.

None yet

5 participants