Skip to content

Refactor Python binding#590

Merged
jiazhihao merged 100 commits intomasterfrom
python_refactor
Jul 15, 2023
Merged

Refactor Python binding#590
jiazhihao merged 100 commits intomasterfrom
python_refactor

Conversation

@eddy16112
Copy link
Copy Markdown
Collaborator

@eddy16112 eddy16112 commented Jan 25, 2023

Description of changes:

This is a big refactor of python binding. We do not build our own flexflow_python, instead, we rely on the legion_python provided by Legion. There are two major benefits:

  1. We can seamlessly switch between legion python and native python interpreter. If we run
legion_python flexflow_script.py

FlexFlow will detect that legion python is used, and use legion python, if we run

python flexflow_script.py

FlexFlow will detect that legion python is not used, so fall back to use native python.

  1. FlexFlow now will work with other Legion python libraries such as Legate.

TODOs:

  • fix makefile
  • factor the cpp code such that we generate a single libflexflow.so instead of a libflexflow.so and libflexflow_c.so
  • fix the initialization of FlexFlow::FFConfig. Currently we get machine configuration such as number of gpus from argv. Since we are now register tasks and mapper after runtime is started, we are not able to get machine configurations from argv because once Legion consumes -ll:gpu, it will be removed from argv, and FlexFlow can not get the value from the argv.

Related Issues:

Linked Issues:

  • Issue #

Issues closed by this PR:

Before merging:

  • Did you update the flexflow-third-party repo, if modifying any of the Cmake files, the build configs, or the submodules?

@eddy16112
Copy link
Copy Markdown
Collaborator Author

Both CMake and Makefile work now, we just need to fix the make install for CMake, feel free to take a look at it.

@eddy16112
Copy link
Copy Markdown
Collaborator Author

Right now, everything seems to work if we apply the following changes to Legion: see here. To get all tests to work in this PR, we will have to either merge PR 838 in Legion, or change our cmake files and tests to always install FlexFlow, instead of building it and running in place. This is because bindings/python/CMakeLists.txt does not currently have code to build the Legion Python bindings without installing them

Can we do make install? Legion's cmake will install the legion files into the conda env.

@goliaro
Copy link
Copy Markdown
Collaborator

goliaro commented Jul 12, 2023

Right now, everything seems to work if we apply the following changes to Legion: see here. To get all tests to work in this PR, we will have to either merge PR 838 in Legion, or change our cmake files and tests to always install FlexFlow, instead of building it and running in place. This is because bindings/python/CMakeLists.txt does not currently have code to build the Legion Python bindings without installing them

Can we do make install? Legion's cmake will install the legion files into the conda env.

I think so! But why should we change FlexFlow so that its python bindings are only built if we install it somewhere, and remove the option to run FlexFlow without installing? With the changes in the bindings/python/CMakeLists.txt we can always run make install and install everything into the conda env as you said, but we can also build in place and run FlexFlow, with or without conda. We can also build multiple versions of FlexFlow on the same machine (e.g. when working on several projects on different branches) without having to create and activate a separate conda environment for each of them

@eddy16112
Copy link
Copy Markdown
Collaborator Author

Right now, everything seems to work if we apply the following changes to Legion: see here. To get all tests to work in this PR, we will have to either merge PR 838 in Legion, or change our cmake files and tests to always install FlexFlow, instead of building it and running in place. This is because bindings/python/CMakeLists.txt does not currently have code to build the Legion Python bindings without installing them

Can we do make install? Legion's cmake will install the legion files into the conda env.

I think so! But why should we change FlexFlow so that its python bindings are only built if we install it somewhere, and remove the option to run FlexFlow without installing?

I think your PR just runs setup.py as the post make operation, which is the same as make install. You are just make the make as make install, there is no difference.

@jiazhihao
Copy link
Copy Markdown
Collaborator

Let's discuss this PR during the FF meeting today.

@jiazhihao jiazhihao merged commit 77fc0b8 into master Jul 15, 2023
@goliaro goliaro deleted the python_refactor branch July 15, 2023 15:03
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.

flexflow_python built in python instead of the build directory

4 participants