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

ERROR: MethodError: no method matching result_style(::Type{StaticArray}) #347

Closed
CodeLenz opened this issue Nov 28, 2017 · 17 comments
Closed

Comments

@CodeLenz
Copy link

Hi

found a new issue today (my code was running without any issues, so it must be something related to julia itself)

Julia Version 0.7.0-DEV.2675
Commit f383276c5b (2017-11-27 20:28 UTC)
Platform Info:
OS: Linux (x86_64-redhat-linux)
CPU: Intel(R) Xeon(R) CPU E5-2630 v4 @ 2.20GHz
WORD_SIZE: 64
BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
LAPACK: libopenblas64_
LIBM: libopenlibm
LLVM: libLLVM-3.9.1 (ORCJIT, broadwell)

and a fresh clone from this repository.

The error:

julia> A = @SMatrix ones(2,2)
2×2 SArray{Tuple{2,2},Float64,2,4}:
 1.0  1.0
 1.0  1.0

julia> A*0.1
ERROR: MethodError: no method matching result_style(::Type{StaticArray})
Closest candidates are:
  result_style(::Any, ::Any) at broadcast.jl:273
  result_style(::Base.Broadcast.BroadcastStyle) at broadcast.jl:270
  result_style(::S<:Base.Broadcast.BroadcastStyle, ::S<:Base.Broadcast.BroadcastStyle) where S<:Base.Broadcast.BroadcastStyle at broadcast.jl:271
Stacktrace:
 [1] combine_styles(::SArray{Tuple{2,2},Float64,2,4}) at ./broadcast.jl:265
 [2] combine_styles(::SArray{Tuple{2,2},Float64,2,4}, ::Float64) at ./broadcast.jl:266
 [3] *(::SArray{Tuple{2,2},Float64,2,4}, ::Float64) at /home/lenz/.julia/v0.7/StaticArrays/src/linalg.jl:26
 [4] top-level scope

@fredrikekre
Copy link
Member

Broadcasting was reworked just the other day on julia master. In general, I would not rely on packages working on julia master these days.

@CodeLenz
Copy link
Author

OK, thanks. Just pointing the issue in order to help tracking the changes.

@c42f
Copy link
Member

c42f commented Nov 29, 2017

Thanks for reporting this, I think it's a perfectly valid issue so I'm going to reopen it.

Fredrik was rightly pointing out that keeping things working relative to julia master is a hard task for package authors. However, we will need to address the issue you've found at some stage.

@c42f c42f reopened this Nov 29, 2017
@andyferris
Copy link
Member

So... the question with v0.7 is when to fork from v0.6 development (I think this will be easiest).

I'm not seeing much v0.6 StaticArrays development at the moment, and feature freeze for Julia v0.7 should in theory only be weeks away, so I'd suggest we can do this soon.

@c42f
Copy link
Member

c42f commented Nov 29, 2017

I still really hope we can avoid forking the development this time. I don't think a wholesale rewrite will be necessary like it was for 0.6, and I feel the 0.5->0.6 transition was pretty rough for users of this package.

Do you know of any really breaking changes which will force the package to work in a significantly different way?

@andyferris
Copy link
Member

Hmm... Well, I was thinking of rewriting the internals with if @generated everywhere. We’d use the new Base broadcast APIs and any LinAlg changes that make v1.0.

I think the StaticArray interface may stay the same, otherwise.

@c42f
Copy link
Member

c42f commented Nov 29, 2017

If we have a completely compatible interface that will probably work out I suppose. Even so, I feel we should hold off on any big changes in master until 0.7 is out for long enough that we have a solid transitional release. I predict it will be better for users and a lot less bug reports for us to deal with ;-)

@andyferris
Copy link
Member

Yes, less bug reports is good. :)

Regarding the issue at hand - are we thinking of using if VERSION in the broadcast.jl file? Does compat deal with this properly?

@c42f
Copy link
Member

c42f commented Nov 29, 2017

Don't know about Compat, but technically the Base.Broadcast stuff was an internal API. I'm guessing it'll need to be a VERSION check, but that's easy enough.

@pablosanjose
Copy link
Contributor

This issue is ruining my day :-D. Do you think adapting the new broadcast machinery into SparseArrays could be a feasible task for a motivated noob? Or should we leave this to the gurus?

@c42f
Copy link
Member

c42f commented Dec 6, 2017

For sure, please feel free to have a go, I don't think it should be too bad. The Base broadcast machinery is actually documented now, so it should be easier than before. See https://docs.julialang.org/en/latest/manual/interfaces/#man-interfaces-broadcasting-1

Things to look for

  • The relevant existing code is a fairly small amount at the top of broadcast.jl comprising the functions _containertype, promote_containertype, broadcast_indices and broadcast_c. These can to be put behind a version check which you can find by running contrib/commit-name.sh on the julia commit where the new broadcast interface was merged.
  • Ensure that the generated code is good quality. I usually look a lot at @code_native for this - basically you want the assembly for simple operations like foo(v1,v2) = v1 .+ v2 to be compact, involving hopefully just a few vector adds, and certainly no branching or function calls.

@c42f
Copy link
Member

c42f commented Dec 6, 2017

Looking at the Base docs, you'll probably want to bypass the default machinery before Base.broadcast_similar is called. According to the docs that's done by overriding broadcast(f, ::DestStyle, ::Void, ::Void, As...).

@pablosanjose
Copy link
Contributor

Thanks, I'll give it a try, see how far I get

@timholy
Copy link
Member

timholy commented Dec 6, 2017

Probably easier would be to specialize broadcast(f, ::DestStyle, ::Type{ElType}, inds::Tuple, As...).

@c42f
Copy link
Member

c42f commented Dec 7, 2017

That makes sense.

@pablosanjose - the trick here is to ensure that the output shape propagates through all the way so that the result type can be fully inferred. Generally that means you'll need to compute the shape recursively. I expect Tim has set it up so it's pretty easy to slot in what we need for StaticArrays.

@pablosanjose
Copy link
Contributor

pablosanjose commented Dec 7, 2017

I gave it a shot. Its a very simple patch, merely reusing the old machinery through the new API. It's perhaps not what you were planning to have long term, but at least broadcast is once more working for me, with one exception (see #348 for details) [EDIT: fixed].

@c42f
Copy link
Member

c42f commented Jul 31, 2019

This was solved quite some time ago!

@c42f c42f closed this as completed Jul 31, 2019
@c42f c42f mentioned this issue Oct 22, 2019
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 a pull request may close this issue.

6 participants