accumulate(op, v0, x) fails on arrays of non-numeric types #25506
Comments
accumulate(+,0, [1.1])would fail with the proposed fix. |
jw3126
added a commit
to jw3126/julia
that referenced
this issue
Jan 11, 2018
|
A full solution to this would probably involve extending #25051 to |
jw3126
added a commit
to jw3126/julia
that referenced
this issue
Jan 17, 2018
|
I am facing this issue while trying cumulative counting along the lines of the following: accumulate((acc, c) -> acc + (c == 'a' ? 1 : 0), 0, collect("cdaabcaa")) # ERRORMy workaround for such a case was: accumulate(+, map(c -> c == 'a' ? 1 : 0, collect("cdaabcaa")))I would love to know if there is a better approach to do this! |
|
not yet, unfortunately. |
Keno
added a commit
that referenced
this issue
Mar 19, 2018
This is a bit of a tricky one, so let me start at the beginning. In PR #25506, I pushed my WIP of a new inlining passes. However, one of the things it temporarily doesn't do is turn call sites to :invoke, causing significantly more jl_apply_generic calls to be codegened. On CI for that PR (as well as locally), we non-deterministically see errors like: ``` LoadError("sysimg.jl", 213, KeyError(LibGit2 [76f85450-5226-5b5a-8eaa-529ad045b433])) ``` Some investigation revealed that what happened here is that a `PkgId` got placed in a dictionary, but was later failed to be retrieved because its hash had supposedly changes. Further investigation reveals that while the hash function used to place the item in the dictionary was `hash(x::Union{String,SubString{String}}, h::UInt64)` in hashing2.jl, the hash function being used to retrieve it is `hash(@nospecialize(x), h::UInt64)` in hashing.jl. The former is loaded later than the latter and should thus invalidate all specializations of the latter, making them ineligible for selection by jl_apply_generic. However, looking at the appropriate age ranges showed that `typeof(hash).name.mt.cache` had entries for both hash functions with overlapping age ranges (which is obviously incorrect). Tracking age range updates, it turns out the world age range for the specialization of `hash(@nospecialize(x), h::UInt64)` was expanded by invalidate_backedges called upon the definition of a hash function inside `LibGit2`. This seems wrong. I believe invalidate_backedges should only ever truncate world age ranges rather than expanding them. This patch simply does that. The non-determinism of the original issue appears to have to do with which of the specializations happen to be in the `jl_lookup_generic_` `call_cache` fast-path. This issue is fairly hard to reproduce because it requires a very specific confluence of circumstances. Since the range of the captured specialization only gets extended to before the min_world age of the new definition, it is never visible in the latest world (e.g. at the REPL). The included test case demonstrates the issue by capturing the world age with a task. Commenting out the `f(x::Float64)` definition makes the test pass, because it is that definition that causes the expansion of the original specialization of `f`.
Keno
added a commit
that referenced
this issue
Mar 19, 2018
This is a bit of a tricky one, so let me start at the beginning. In PR #25506, I pushed my WIP of a new inlining passes. However, one of the things it temporarily doesn't do is turn call sites to :invoke, causing significantly more jl_apply_generic calls to be codegened. On CI for that PR (as well as locally), we non-deterministically see errors like: ``` LoadError("sysimg.jl", 213, KeyError(LibGit2 [76f85450-5226-5b5a-8eaa-529ad045b433])) ``` Some investigation revealed that what happened here is that a `PkgId` got placed in a dictionary, but was later failed to be retrieved because its hash had supposedly changes. Further investigation reveals that while the hash function used to place the item in the dictionary was `hash(x::Union{String,SubString{String}}, h::UInt64)` in hashing2.jl, the hash function being used to retrieve it is `hash(@nospecialize(x), h::UInt64)` in hashing.jl. The former is loaded later than the latter and should thus invalidate all specializations of the latter, making them ineligible for selection by jl_apply_generic. However, looking at the appropriate age ranges showed that `typeof(hash).name.mt.cache` had entries for both hash functions with overlapping age ranges (which is obviously incorrect). Tracking age range updates, it turns out the world age range for the specialization of `hash(@nospecialize(x), h::UInt64)` was expanded by invalidate_backedges called upon the definition of a hash function inside `LibGit2`. This seems wrong. I believe invalidate_backedges should only ever truncate world age ranges rather than expanding them. This patch simply does that. The non-determinism of the original issue appears to have to do with which of the specializations happen to be in the `jl_lookup_generic_` `call_cache` fast-path. This issue is fairly hard to reproduce because it requires a very specific confluence of circumstances. Since the range of the captured specialization only gets extended to before the min_world age of the new definition, it is never visible in the latest world (e.g. at the REPL). The included test case demonstrates the issue by capturing the world age with a task. Commenting out the `f(x::Float64)` definition makes the test pass, because it is that definition that causes the expansion of the original specialization of `f`.
simonbyrne
pushed a commit
that referenced
this issue
Mar 29, 2018
StefanKarpinski
pushed a commit
that referenced
this issue
Apr 5, 2018
ararslan
pushed a commit
that referenced
this issue
Apr 26, 2018
This is a bit of a tricky one, so let me start at the beginning. In PR #25506, I pushed my WIP of a new inlining passes. However, one of the things it temporarily doesn't do is turn call sites to :invoke, causing significantly more jl_apply_generic calls to be codegened. On CI for that PR (as well as locally), we non-deterministically see errors like: ``` LoadError("sysimg.jl", 213, KeyError(LibGit2 [76f85450-5226-5b5a-8eaa-529ad045b433])) ``` Some investigation revealed that what happened here is that a `PkgId` got placed in a dictionary, but was later failed to be retrieved because its hash had supposedly changes. Further investigation reveals that while the hash function used to place the item in the dictionary was `hash(x::Union{String,SubString{String}}, h::UInt64)` in hashing2.jl, the hash function being used to retrieve it is `hash(@nospecialize(x), h::UInt64)` in hashing.jl. The former is loaded later than the latter and should thus invalidate all specializations of the latter, making them ineligible for selection by jl_apply_generic. However, looking at the appropriate age ranges showed that `typeof(hash).name.mt.cache` had entries for both hash functions with overlapping age ranges (which is obviously incorrect). Tracking age range updates, it turns out the world age range for the specialization of `hash(@nospecialize(x), h::UInt64)` was expanded by invalidate_backedges called upon the definition of a hash function inside `LibGit2`. This seems wrong. I believe invalidate_backedges should only ever truncate world age ranges rather than expanding them. This patch simply does that. The non-determinism of the original issue appears to have to do with which of the specializations happen to be in the `jl_lookup_generic_` `call_cache` fast-path. This issue is fairly hard to reproduce because it requires a very specific confluence of circumstances. Since the range of the captured specialization only gets extended to before the min_world age of the new definition, it is never visible in the latest world (e.g. at the REPL). The included test case demonstrates the issue by capturing the world age with a task. Commenting out the `f(x::Float64)` definition makes the test pass, because it is that definition that causes the expansion of the original specialization of `f`. Ref #26514 (cherry picked from commit 90a2162)
ararslan
pushed a commit
that referenced
this issue
Apr 26, 2018
This is a bit of a tricky one, so let me start at the beginning. In PR #25506, I pushed my WIP of a new inlining passes. However, one of the things it temporarily doesn't do is turn call sites to :invoke, causing significantly more jl_apply_generic calls to be codegened. On CI for that PR (as well as locally), we non-deterministically see errors like: ``` LoadError("sysimg.jl", 213, KeyError(LibGit2 [76f85450-5226-5b5a-8eaa-529ad045b433])) ``` Some investigation revealed that what happened here is that a `PkgId` got placed in a dictionary, but was later failed to be retrieved because its hash had supposedly changes. Further investigation reveals that while the hash function used to place the item in the dictionary was `hash(x::Union{String,SubString{String}}, h::UInt64)` in hashing2.jl, the hash function being used to retrieve it is `hash(@nospecialize(x), h::UInt64)` in hashing.jl. The former is loaded later than the latter and should thus invalidate all specializations of the latter, making them ineligible for selection by jl_apply_generic. However, looking at the appropriate age ranges showed that `typeof(hash).name.mt.cache` had entries for both hash functions with overlapping age ranges (which is obviously incorrect). Tracking age range updates, it turns out the world age range for the specialization of `hash(@nospecialize(x), h::UInt64)` was expanded by invalidate_backedges called upon the definition of a hash function inside `LibGit2`. This seems wrong. I believe invalidate_backedges should only ever truncate world age ranges rather than expanding them. This patch simply does that. The non-determinism of the original issue appears to have to do with which of the specializations happen to be in the `jl_lookup_generic_` `call_cache` fast-path. This issue is fairly hard to reproduce because it requires a very specific confluence of circumstances. Since the range of the captured specialization only gets extended to before the min_world age of the new definition, it is never visible in the latest world (e.g. at the REPL). The included test case demonstrates the issue by capturing the world age with a task. Commenting out the `f(x::Float64)` definition makes the test pass, because it is that definition that causes the expansion of the original specialization of `f`. Ref #26514 (cherry picked from commit 90a2162)
ararslan
pushed a commit
that referenced
this issue
Apr 27, 2018
This is a bit of a tricky one, so let me start at the beginning. In PR #25506, I pushed my WIP of a new inlining passes. However, one of the things it temporarily doesn't do is turn call sites to :invoke, causing significantly more jl_apply_generic calls to be codegened. On CI for that PR (as well as locally), we non-deterministically see errors like: ``` LoadError("sysimg.jl", 213, KeyError(LibGit2 [76f85450-5226-5b5a-8eaa-529ad045b433])) ``` Some investigation revealed that what happened here is that a `PkgId` got placed in a dictionary, but was later failed to be retrieved because its hash had supposedly changes. Further investigation reveals that while the hash function used to place the item in the dictionary was `hash(x::Union{String,SubString{String}}, h::UInt64)` in hashing2.jl, the hash function being used to retrieve it is `hash(@nospecialize(x), h::UInt64)` in hashing.jl. The former is loaded later than the latter and should thus invalidate all specializations of the latter, making them ineligible for selection by jl_apply_generic. However, looking at the appropriate age ranges showed that `typeof(hash).name.mt.cache` had entries for both hash functions with overlapping age ranges (which is obviously incorrect). Tracking age range updates, it turns out the world age range for the specialization of `hash(@nospecialize(x), h::UInt64)` was expanded by invalidate_backedges called upon the definition of a hash function inside `LibGit2`. This seems wrong. I believe invalidate_backedges should only ever truncate world age ranges rather than expanding them. This patch simply does that. The non-determinism of the original issue appears to have to do with which of the specializations happen to be in the `jl_lookup_generic_` `call_cache` fast-path. This issue is fairly hard to reproduce because it requires a very specific confluence of circumstances. Since the range of the captured specialization only gets extended to before the min_world age of the new definition, it is never visible in the latest world (e.g. at the REPL). The included test case demonstrates the issue by capturing the world age with a task. Commenting out the `f(x::Float64)` definition makes the test pass, because it is that definition that causes the expansion of the original specialization of `f`. Ref #26514 (cherry picked from commit 90a2162)
|
All the examples here don't produce any error now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following produces an error:
The same happens for
It seems that
accumulate(op, v0, x)tries to guess the type of the output array based on both the type of the initial value and the type of the input array, while it should only consider the first.Changing this line:
julia/base/multidimensional.jl
Line 1105 in 4440b0f
to:
seems to fix the issue, but I am not sure whether there is a reason to prefer the original version.
The text was updated successfully, but these errors were encountered: