Skip to content

Conversation

@avik-pal
Copy link
Collaborator

@avik-pal avik-pal commented Feb 5, 2025

No description provided.

@avik-pal

This comment was marked as outdated.

@avik-pal avik-pal changed the title feat: API changes for multi-device execution feat: API changes for multi-device execution [ReactantExtra JLL changes] Feb 5, 2025
@avik-pal avik-pal requested review from giordano and wsmoses and removed request for wsmoses February 5, 2025 20:32
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK for as much as I can tell (which is limited 😅).

still debugging 1 segfault from the finalizer

Was that solved? I don't see new commits after that message.

@avik-pal
Copy link
Collaborator Author

avik-pal commented Feb 5, 2025

Actually wait no not solved. I thought it got fixed... But it does segfault on finalizer. Let me investigate under gdb and hope it reveals more info

@wsmoses
Copy link
Member

wsmoses commented Feb 5, 2025

Not a segfault source but I do see a memory error.

For example: here we allocate memory

return cstr_from_string(client->platform_name());

and when using it
str = @ccall MLIR.API.mlir_c.ClientGetPlatformName(

I don't think we ever free it (we need to do unsafe_string, and then free the memory).

@avik-pal
Copy link
Collaborator Author

avik-pal commented Feb 6, 2025

We can go ahead with this JLL. Turns out I don't understand basic arithmetic 😢

@avik-pal avik-pal merged commit c325683 into main Feb 6, 2025
30 of 32 checks passed
@avik-pal avik-pal deleted the ap/multidevice_jll branch February 6, 2025 15:33
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.

3 participants