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

WIP: Try making loading of mlir dynamic with importlib. #110

Closed
wants to merge 1 commit into from

Conversation

ingomueller-net
Copy link
Contributor

No description provided.

@@ -1,4 +1,5 @@
import mlir.ir
import xdsl.mlir as mlir
Copy link
Collaborator

Choose a reason for hiding this comment

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

I must admit, I don't fully understand how importing works exactly in python, but I am uncertain whether this will actually fix the problem. I am guessing that you would like to have a import mlir_iterators as mlir somewhere in the specific converter, which should overwrite the mlir that is used here to the one we actually want ot use. I did several such attempts trying to reimport a module using the same name and that never worked. And as mentioned, I don't actually understand in what sense this approach differs from mine other than having this extra file, which I don't quite understand the functionality off, since it does not seem to be adjustable to import from somewhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, true, the most interesting part is missing! In Line 1, I just import 'mlir'. What is interesting, is that this string can now be read from anywhere such as environment variables.

But I thought about this again. What I like about your approach is that it shows you don't need a string. I thought that maybe a combination of the two could be the best: keep a global variable, for example, named mlir in src/xdsl/config.py, which is initialized with __import__('mlir', fromlist=['*']). Then, all xDSL code has to use that variable instead of the global module name. This is similar to your class member variable but not bound to a class instance. Users can then still overwrite which module to use by overwriting that variable.

On the other hand, using a string has the advantage that the default mlir package does not need to exist if the string is changed even before attempting to import it.

Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand the problem you currently trying to fix. What do you mean by dynamically loading MLIR?

@ingomueller-net
Copy link
Contributor Author

I'm not sure I understand the problem you currently trying to fix. What do you mean by dynamically loading MLIR?

I tried to explain it more precisely in #111.

@math-fehr
Copy link
Collaborator

Thanks, I understand more the problem now!

@math-fehr
Copy link
Collaborator

I answer my thoughts in #109, and I'm wondering if maybe one solution would be to use a "setup.py" file or something to create an module that would essentially just redirect to the right custom module.
I'm not certain if this is the right way, I'm just bouncing ideas.

@webmiche
Copy link
Collaborator

webmiche commented May 9, 2022

AFAIU, the setup.py is/was used to define requirements and depedencies s.t. a package can acually be installed (Source). I guess we could have people change a line in setup.py, when they want to change the module that they want to link to and then "reinstall" in a sense.

I think my vision of a solution is that there is somewhere a file, where you can change mlir to mlir_*something",which will then load the right thing. That file might be just simply the string and we read that file from somewhere else and import as a string. I'll try to implement something like that.

@ingomueller-net
Copy link
Contributor Author

I guess I see how you can decide in setup.py which dependency you want to install but you still need to change import mlir in your sources. Is your plan to implement a preprocessor and modify the files upon installation? Apart from looking brittle, it sure does not work for your development copy where setup.py isn't used at all...

@webmiche
Copy link
Collaborator

I feel that the setup.py approach won't work or will at least be a huge struggle since it does not seem to be meant to be used like that... 🤔

@webmiche webmiche closed this May 14, 2022
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.

None yet

3 participants