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

Mutable struct bugs #1111

Open
mcabbott opened this issue Oct 27, 2021 · 1 comment
Open

Mutable struct bugs #1111

mcabbott opened this issue Oct 27, 2021 · 1 comment

Comments

@mcabbott
Copy link
Member

Something is currently broken about the handling of mutable structs. One place I can isolate this is the following example, where Ref(Ref(x)) leads silently to wrong answers:

gradient(x -> abs2(x.x) + 7 * real(x.x), Ref(1+im))  # works correctly, ((x = 9.0 + 2.0im,),)

gradient(x -> abs2(x[].x) + 7 * real(x[].x), Ref(Ref(1+im)))  # gives (nothing,)

That has been broken since at least 0.6.0. This version, with Array(Ref(x)), was not broken in 0.6.20, but currently gives a wrong answer:

gradient(x -> abs2(x[1].x) + 7 * real(x[1].x), [Ref(1+im)])  # wrong answer, ([(x = 16.0 + 2.0im,)],)

I presume that #1102 and/or #1103 caused this regression. There were, and are, astonishingly few tests of mutable structs. If anyone has other use cases they think ought to work, and can be compactly summarised to add to tests, that would be extremely helpful.

There are other mysterious bugs which seem like they are related to the handling of mutable structs, for which I don't have a compact example. It's likely that #1109 is related.

Bugs involving only immutable structs are not in scope here. There are some remaining leaks of ChainRules types, every time you fix one another appears... many are fixed by #1104

@lazarusA
Copy link

lazarusA commented Mar 2, 2023

testing this now [Zygote v0.6.55] outputs:

gradient(x -> abs2(x.x) + 7 * real(x.x), Ref(1+im))  # works correctly, ((x = 9.0 + 2.0im,),)
gradient(x -> abs2(x[].x) + 7 * real(x[].x), Ref(Ref(1+im)))  # gives (nothing,)
((x = 9.0 + 2.0im,),)

((x = (x = 9.0 + 2.0im,),),)

it looks ok, besides the obvious extra nesting.

I'm about to go this road, using mutable struct's. Docs said, not use if possible, but life is/will be much easier using them if they work as expected 😄 .

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

No branches or pull requests

2 participants