-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Error constructing a multivector #7
Comments
That is actually not a consequential error, because you still get the result at the end, and if you evaluate the input another time you do not get any error. this is very strange indeed, and I have noticed this behavior before also in some rare cases, it would help to know what exactly causes it.. Thanks for sharing your version of the error message, on my computer it is like this julia> using Grassmann
julia> basis"+++"
(⟨+++⟩, v, v₁, v₂, v₃, v₁₂, v₁₃, v₂₃, v₁₂₃)
julia> v+v1+v2
Internal error: encountered unexpected error in runtime:
MethodError(f=typeof(Base.string)(), args=(Expr(:call, Symbol("!"), Expr(:., :typ, :(:mutable))),), world=0x0000000000000eb9)
unknown function (ip: 0x7f0edd30ed58)
jl_throw at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0edd2c69fa)
unknown function (ip: 0x7f0edd2c6b19)
jl_apply_generic at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0ed1fd6525)
unknown function (ip: 0x7f0ed1fde2ee)
unknown function (ip: 0x7f0ed2011f97)
unknown function (ip: 0x7f0ed2013025)
unknown function (ip: 0x7f0ed20143ae)
unknown function (ip: 0x7f0ed2015300)
unknown function (ip: 0x7f0ed2015733)
unknown function (ip: 0x7f0ed2015b00)
jl_apply_generic at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0edd2c8721)
unknown function (ip: 0x7f0edd2c907e)
jl_fptr_trampoline at /usr/bin/../lib/libjulia.so.1 (unknown line)
jl_apply_generic at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0edd421057)
unknown function (ip: 0x7f0edd420d92)
unknown function (ip: 0x7f0edd421a4f)
unknown function (ip: 0x7f0edd422144)
Interpreter frame (ip: 0)
Core.CodeInfo(code=Array{Any, (2,)}[
Expr(:call, :+, :v, :v1, :v2),
Expr(:return, SSAValue(1))], codelocs=Array{Int32, (2,)}[1, 1], method_for_inference_limit_heuristics=nothing, ssavaluetypes=2, linetable=Array{Any, (1,)}[Core.LineInfoNode(mod=Main, method=Symbol("top-level scope"), file=:none, line=0, inlined_at=0)], ssaflags=Array{UInt8, (0,)}[], slotflags=Array{UInt8, (0,)}[], slotnames=Array{Any, (0,)}[], inferred=false, inlineable=false, propagate_inbounds=false, pure=false)unknown function (ip: 0x7f0edd42260c)
unknown function (ip: 0x7f0edd2f979c)
jl_toplevel_eval_in at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0ed2050f51)
jl_apply_generic at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0ed21e83f5)
unknown function (ip: 0x7f0ed21e865b)
jl_apply_generic at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7f0edd2e0617)
unknown function (ip: 0xffffffffffffffff)
1 + 1v₁ + 1v₂
julia> v+v1+v2
1 + 1v₁ + 1v₂ as you can see, the error does not pose any real problem yet, since the result still works |
I'm labeling this won't fix, because I'm not entirely sure what is going on with this at the moment yet it does not seem to actually cause any issues with obtaining any results though |
Removing all uses of |
Just to reiterate: At the very least (looking through the quoted parts) there are A LOT of places where
|
There are many situations where I used In some cases, it may not look as a valid usage of This is going to require a more precise analysis, which is going to be very difficult for non-authors to be able to do independently since they do not know of the implicit intentions and principles used in this code. Also, I am working in making foundational changes to the design of the code constantly, so this package is not in a stable state yet... this is v0.0.x |
Almost no non-trivial Julia method definitions are actually Again, why do you feel it's necessary to use the non-exported |
In this particular package, the entire design constraint is centered around passing around parametric type data ahead of compile time in order to make the generated code more efficient, yet also remain fully flexible for both numerical and symbolic computation with many different dimensional spin algebras. That is why if you naively remove all usage of The usage of it probably needs to be investigated more, but it is part of the fundamental design here. |
That sounds like it might be a better use case for generated functions. |
In https://github.com/eschnett/FastArrays.jl, I am using generated functions in many places. (Recently, Julia's compiler became better, so after benchmarking I am able to convert some generated functions to generic functions.) I do this for the same reason: I need these functions to be fast, and any kind of genericity that still remains at run time has an unacceptable performance cost. If the compiler completely specializes functions based on rank, dimension, types, etc., then LLVM can optimize and inline away many operations. For example, if you have a (3 x N) array, and the compiler does not know that the first dimension is 3, then there needs to be an integer multiplication at run time. If the compiler does know, however, then the generated code is becomes much more efficient. I assume similar issues are true for this package, and I'd also assume that generated functions are the way to go. |
Let's make this discussion a bit more concrete by recalling the example cited from my discussion post Since then, I have significantly changed the implementation of this particular example in this repository already, but this outdated version from the post makes for a good continued discussion example: struct data{N,G} end
const binomsum_cache = [[1]];
Base.@pure function pure_binomsum(n::Int, i::Int)
for k=length(binomsum_cache)+1:n
push!(binomsum_cache, cumsum([binomial(k,q) for q=0:k]))
end
i!=0 ? binomsum_cache[n][i] : 0
end Which is then used to define a test function, for which we require precompiled pure(::data{N,G}) where {N,G} = float(pure_binomsum(N,G)); What I want is for the code to look like this julia> @code_warntype pure(data{5,4}())
Body::Float64
1 1 ─ return 26.0 However, as demonstrated in my discussion post, using To move the discussion forward, it would be best if you could demonstrate your proposals with this test. |
This becomes: struct data{N,G} end
const binomsum_cache = [[1]]
@generated function binomsum(::Val{n}, ::Val{i}) where {n, i}
for k=length(binomsum_cache)+1:n
push!(binomsum_cache, cumsum([binomial(k,q) for q=0:k]))
end
res = i!=0 ? binomsum_cache[n][i] : 0
quote
$res
end
end
pure(::data{N,G}) where {N,G} = float(binomsum(Val(N), Val(G))) with julia> @code_warntype pure(data{5,4}())
Body::Float64
1 ─ return 26.0 The main trick with generated functions is that one essentially writes a macro, but the types of the function arguments are known. One can use |
... since thread-safety was mentioned somewhere: I don't think the code above is thread-safe. One needs a mutex around the for loop that pushes to the global variable. Since this for loop is only executed at compile time, this does not affect run-time performance. |
Actually, the thread safety has also already been completely addressed since then, in another separate follow up discussion, where it turned out that the thread-safety issue was caused by a logging message that has been completely removed since, and now is now resolved. Thank you for your example, I will take that into consideration for the next release, which will have a variety of planned updates and modifications. |
@StefanKarpinski @eschnett your solution is unfortunately too much of a degradation in performance pur_summage(x) = sum([pure_binomsum(x,y) for y ∈ 0:x])
gen_summage(x) = sum([binomsum(Val(x),Val(y)) for y ∈ 0:x]) this julia> @btime pur_summage(7)
69.492 ns (1 allocation: 144 bytes)
448
julia> @btime gen_summage(7)
48.298 μs (3 allocations: 208 bytes)
448 Perhaps also @JeffBezanson or @vtjnash could comment on this. If there is something the Julia language could improve to solve this, it would be to have some kind of softer Perhaps, this neeeds an entirely new macro name, because it is not |
The difference in performance between the pure and generated functions is all due to dynamic dispatch of the generated function in the loop. For this function, you don't actually need struct data{N,G} end
const binomsum_cache = [[1]];
Base.@pure function pure_binomsum(n::Int, i::Int)
for k=length(binomsum_cache)+1:n
push!(binomsum_cache, cumsum([binomial(k,q) for q=0:k]))
end
i!=0 ? binomsum_cache[n][i] : 0
end
function reg_binomsum(n::Int, i::Int)
for k=length(binomsum_cache)+1:n
push!(binomsum_cache, cumsum([binomial(k,q) for q=0:k]))
end
i!=0 ? binomsum_cache[n][i] : 0
end
pur_summage(x) = sum([pure_81.525 ns(x,y) for y ∈ 0:x])
reg_summage(x) = sum([reg_binomsum(x,y) for y ∈ 0:x])
julia> @btime pur_summage(7);
81.026 ns (1 allocation: 144 bytes)
julia> @btime reg_summage(7)
81.525 ns The reason for this is that you've already done an (unsafe due to constant redefinition) version of memoization in the actual definition of |
Thank you for the benchmarks, but the purpose of julia> @code_warntype pure(data{5,4}())
Body::Float64
1 ─ return 26.0 That's why the function needs a macro like that. My point: however, maybe a different macro is needed, one which has much less strict requirements than Perhaps this macro does not exist yet, and needs to be added to the language? It needs both performance, and needs to be able to be precomputed ahead of time from type data. |
The point that I believe you are missing is that the function already is precomputed ahead of time without the The first time one computes |
You are right about that, but it is irrelevant to what I'm talking about. Whether the value is memoized or not is completely irrelevant to obtaining code like this: julia> @code_warntype pure(data{5,4}())
Body::Float64
1 ─ return 26.0 You only get that kind of code when you use |
Yes, instead the code it produces is essentially Caching a value in a lookup table or using |
You don't understand the point of the test function. The point of it is not to cache a value, but the point is to test the ability of the compiler to simplify the result ahead of time based on The fact that I have a cache built-in is simply an extra fact added onto it, but is not relevant to whether I chose to use |
In the code discussed here, certain calculations happen at compile time, and others happen at run time. The memoization lookup table is only there to speed up compiling the code. It is likely not very important whether compiling the code is a few times faster or slower. What matters is that, at run time, only the absolutely necessary calculations are performed: here actually none at all, as the result In my example above I use Are functions such as |
@chakravala In hindsight, an internal function such as for binomials might not have been a good choice to demonstrate how to use generated functions. Do you have an example of a function that is exported and which uses |
The Due to the exponential nature of these algebras, these small differences can make a big difference in how fast your algebra will be at very high dimensions. How deep must the layering of |
I didn't expect that code generation would itself be a bottleneck; I have no experience with this. Generally, all the function that need to be efficient at run time need to be generated. Functions that are only called at compile time don't need to be generated. Standard software engineering (lookup tables, conditionals, etc.) should suffice there. I am not familiar with how |
In response to the other post, the https://github.com/chakravala/Grassmann.jl/blob/master/src/utilities.jl#L170 The |
The algebras I am looking at sometimes require |
This issue is now being closed, because the original reported behavior now appears to be fixed. julia> using Grassmann
julia> basis"+++"
(⟨+++⟩, v, v₁, v₂, v₃, v₁₂, v₁₃, v₂₃, v₁₂₃)
julia> v + v1 + v2
1 + 1v₁ + 1v₂ this is on the current don't know what it is that exactly caused the error, but I've changed around some of the fundamental operations on multivectors in the last few commits and that seems to have fixed it |
The text was updated successfully, but these errors were encountered: