Patches for Lux integration #7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
avik-pal
force-pushed
the
ap/patch
branch
3 times, most recently
from
3186dee to
ab01055
Compare
28 tasks
wsmoses
reviewed
May 27, 2024
Done |
wsmoses
reviewed
May 27, 2024
wsmoses
reviewed
May 27, 2024
| @@ -7,7 +7,7 @@ function transpose_val(val) | |||
| return MLIR.IR.result(MLIR.Dialects.stablehlo.transpose(val, permutation=attr), 1) | |||
| end | |||
|
|
|||
| function apply(f, args...; kwargs) | |||
| function apply(f, args...; kwargs...) | |||
There was a problem hiding this comment.
Can you add a test for what needed this. I somehow recall the other way being intentional but maybe not
There was a problem hiding this comment.
This came up in computing the gradient of the model with Enzyme. But that code is not completely functional yet, see LuxDL/Lux.jl#665 (comment)
|
Ah looking at that error I think we should trace the primal and shadow
arrays into tracedrarrays so then the duplicates invariant holds. We'll
need the data as an rarray anyways to run it
…On Mon, May 27, 2024, 11:33 PM Avik Pal ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/utils.jl
<#7 (comment)>:
> @@ -7,7 +7,7 @@ function transpose_val(val)
return MLIR.IR.result(MLIR.Dialects.stablehlo.transpose(val, permutation=attr), 1)
end
-function apply(f, args...; kwargs)
+function apply(f, args...; kwargs...)
This came up in computing the gradient of the model with Enzyme. But that
code is not completely functional yet, see LuxDL/Lux.jl#665 (comment)
<LuxDL/Lux.jl#665 (comment)>
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJTUXFWCLE53NPEVDMVQGLZEORCZAVCNFSM6AAAAABIKKBQG6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOBRGQ4DANZQGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
However separate from this fix (which ought be done regardless) it is
surprising that the original model is not an rarray as I would expect. That
should probably be fixed in lux
…On Mon, May 27, 2024, 11:36 PM Billy Moses ***@***.***> wrote:
Ah looking at that error I think we should trace the primal and shadow
arrays into tracedrarrays so then the duplicates invariant holds. We'll
need the data as an rarray anyways to run it
On Mon, May 27, 2024, 11:33 PM Avik Pal ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/utils.jl
> <#7 (comment)>:
>
> > @@ -7,7 +7,7 @@ function transpose_val(val)
> return MLIR.IR.result(MLIR.Dialects.stablehlo.transpose(val, permutation=attr), 1)
> end
>
> -function apply(f, args...; kwargs)
> +function apply(f, args...; kwargs...)
>
> This came up in computing the gradient of the model with Enzyme. But that
> code is not completely functional yet, see LuxDL/Lux.jl#665 (comment)
> <LuxDL/Lux.jl#665 (comment)>
>
> —
> Reply to this email directly, view it on GitHub
> <#7 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJTUXFWCLE53NPEVDMVQGLZEORCZAVCNFSM6AAAAABIKKBQG6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOBRGQ4DANZQGY>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
|
wsmoses
reviewed
May 28, 2024
wsmoses
approved these changes
May 28, 2024
avik-pal
commented
May 29, 2024
wsmoses
approved these changes
May 29, 2024
wsmoses
reviewed
May 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am pulling out the patches from #5, so that we can get them in faster