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

Change order of function definition in matrix multiplication code #833

Merged

Conversation

mateuszbaran
Copy link
Collaborator

This is a bugfix for a bug introduced in #814 . In some circumstances generated functions from matrix_multiply_add.jl don't see methods of gen_by_access and uplo_access defined in matrix_multiply.jl (generated functions somehow get instantiated in an earlier world age). This moves all required methods so that they are defined earlier. Unfortunately I don't know how to make a test for this.

Thanks @krzysztofrzecki for discovering this problem!

This should safeguard against generated functions being generated earlier than method definitions they need.
@KristofferC
Copy link
Contributor

(generated functions somehow get instantiated in an earlier world age)

The generator in a generated function only has access to methods in the world age of when the method is defined.

@mateuszbaran
Copy link
Collaborator Author

Yes, the part that I don't understand is why did it work before 😄 . One of the many mysteries of generated functions.

@c42f
Copy link
Member

c42f commented Sep 29, 2020

Yes, the part that I don't understand is why did it work before

As far as I know precompiled modules are loaded into the main session as a single world age. That is, functions within the module behave as if they were all defined at the same time.

So the order shouldn't matter unless the generated function is actually called during precompilation itself.

In what circumstances did the bug manifest?

Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

Looks fine to me btw. Would be nice to know why it's necessary, but 🤷‍♂️

@KristofferC
Copy link
Contributor

Maybe by running with --compiled-modules=no?

@c42f
Copy link
Member

c42f commented Sep 29, 2020

Ah of course, that would do it!

I knew I'd encountered this kind of thing before in some package. I remember now it was Uniful.jl and there was a need to run with --compiled-modules=no due to some pycall problem on windows, or some such thing.

@mateuszbaran
Copy link
Collaborator Author

In what circumstances did the bug manifest?

Matrix multiplication was invoked from pyjulia with compiled_modules=False so it looks like that's it 👍 .

@mateuszbaran mateuszbaran merged commit 301e082 into JuliaArrays:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants