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

setu does not perform any sizecheck #82

Open
hexaeder opened this issue May 31, 2024 · 1 comment · May be fixed by #83
Open

setu does not perform any sizecheck #82

hexaeder opened this issue May 31, 2024 · 1 comment · May be fixed by #83
Labels
bug Something isn't working

Comments

@hexaeder
Copy link

Describe the bug 🐞

setu does not perform sizecheck between the new values and the number of indexed elements.

Expected behavior

I would expect it to either error or implicitly broadcast, but it only overwrites a single value. The expectation comes especially when using the setindex! interface, because then it feels like array indexing

a = zeros(2)
a[[1,2]] = 1 # errors
a[[1,2]] .= 1 # overwrites both elements
a[[1,2]] = [1,2] # overwrites both elements

Minimal Reproducible Example 👇

inpr = SymbolCache([:x, :y])
ps = ProblemState(;u=zeros(2))
    
getu(inpr, [:x,:y])(ps) == zeros(2) # as expected
setu(inpr, [:x,:y])(ps, 1) # does not error, only overwrites first element
getu(inpr, [:x,:y])(ps) == [1,0] # strange?
@hexaeder hexaeder added the bug Something isn't working label May 31, 2024
@hexaeder
Copy link
Author

hexaeder commented Jun 3, 2024

I think this could be solved by replacing map with broadcast here:

function (ms::MultipleSetters)(prob, val)
map((s!, v) -> s!(prob, v), ms.setters, val)
end

Underlying issue being that mapping over multiple collections only loops through the shortest list. I think broadcast behaviour is what you'd expect here

julia> map((a,b)->a*repr(b), ["foo", "bar"], 1)
1-element Vector{String}:
 "foo1"

julia> map((a,b)->a*repr(b), ["foo", "bar"], [1,2])
2-element Vector{String}:
 "foo1"
 "bar2"

julia> map((a,b)->a*repr(b), ["foo", "bar"], [1,2,3])
2-element Vector{String}:
 "foo1"
 "bar2"

julia> broadcast((a,b)->a*repr(b), ["foo", "bar"], 1)
2-element Vector{String}:
 "foo1"
 "bar1"

julia> broadcast((a,b)->a*repr(b), ["foo", "bar"], [1,2])
2-element Vector{String}:
 "foo1"
 "bar2"

julia> broadcast((a,b)->a*repr(b), ["foo", "bar"], [1,2,3])
ERROR: DimensionMismatch: arrays could not be broadcast to a common size: a has axes Base.OneTo(2) and b has axes 

hexaeder added a commit to hexaeder/SymbolicIndexingInterface.jl that referenced this issue Jun 3, 2024
hexaeder added a commit to hexaeder/SymbolicIndexingInterface.jl that referenced this issue Jun 3, 2024
@hexaeder hexaeder linked a pull request Jun 3, 2024 that will close this issue
5 tasks
hexaeder added a commit to hexaeder/SymbolicIndexingInterface.jl that referenced this issue Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant