Skip to content

Allow DefaultArrayStyle to be broadcast according to normal rules #138

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

Merged
merged 2 commits into from
Apr 17, 2021

Conversation

jdeldre
Copy link
Contributor

@jdeldre jdeldre commented Apr 17, 2021

Hot fix to #137

The original issue lies in the way broadcasting works for zeros(), since zeros().+2.0 gets turned from Array{Float64, 0} into Float64, because apparently that is how broadcasting is set up for DefaultArrayStyle{0} (This seems like a bug in Julia.) Before #136, this wasn't a problem for broadcasting of ArrayPartition as long as an Array{Float64, 0} was paired with another component of higher Array rank, e.g., ap = ArrayPartition(zeros(),[1.0]), since they would all be broadcast via DefaultArrayStyle{1} rules. So ap+1.0 would be of the same type as ap. But on its own, e.g., ap = ArrayPartition(zeros()), then ap + 1.0 would not be the same type as ap, since it gets broadcast as a DefaultArrayStyle{0}. Unfortunately, #136 exposed this issue.

This PR provides a more narrowly defined scope for when components of ArrayPartition will change their style in unpack from that of the promoted style of the overall Broadcasted wrapper of the ArrayPartition. If it is of DefaultArrayStyle, then it keeps the overall wrapper's style, thereby preserving the behavior before #136. But if the ArrayPartition contains a custom type with its own BroadcastStyle, then it parses the components' styles and therefore still allows specialized dispatch on copy.

@YingboMa YingboMa merged commit 668e445 into SciML:master Apr 17, 2021
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.

2 participants