Draft: Build OR shared libraries for PIP distribution #2311

QuantamHD · 2022-09-27T18:26:39Z

No description provided.

Signed-off-by: Ethan Mahintorabi <[email protected]>

proppy · 2022-09-27T21:46:03Z

src/cmake/swig_lib.cmake

  set_property(SOURCE ${ARG_I_FILE}
               PROPERTY USE_SWIG_DEPENDENCIES TRUE)
  set_property(SOURCE ${ARG_I_FILE}
               PROPERTY USE_TARGET_INCLUDE_DIRECTORIES true)

+
+  if (ARG_SHARED_LIBRARY)


https://cmake.org/cmake/help/latest/module/UseSWIG.html seems to have a USE_BUILD_SHARED_LIBS option to achieve something similar.

proppy · 2022-09-27T21:49:11Z

src/cmake/swig_lib.cmake

@@ -62,16 +62,29 @@ function(swig_lib)
                                        -Werror
                                        -w317,325,378,401,402,467,472,503,509)

-  set_property(SOURCE ${ARG_I_FILE}


should swig be able to infer this from the %module decl

proppy · 2022-09-27T21:53:52Z

python/setup.py

+from setuptools.command.build_ext import build_ext as build_ext_orig
+
+
+class CMakeExtension(Extension):


what about reusing something like https://github.com/diegoferigo/cmake-build-extension?

proppy · 2022-09-27T21:59:01Z

src/CMakeLists.txt

+  ${OPENROAD_TOOL_LIBS}
+)
+
+target_link_libraries(openroad


should this be libopenroad? (or removed)

ah my bad, that's for the executable.

proppy · 2022-09-27T22:01:10Z

src/CMakeLists.txt

@@ -293,6 +297,24 @@ target_link_libraries(openroad
  ${CMAKE_THREAD_LIBS_INIT}
 )

+add_library(libopenroad


should this also be linked against the optional import below (zlib, cudd, etc)

proppy · 2022-09-27T22:02:29Z

src/CMakeLists.txt

@@ -293,6 +297,24 @@ target_link_libraries(openroad
  ${CMAKE_THREAD_LIBS_INIT}
 )

+add_library(libopenroad
+  ${OPENROAD_LIB_SRCS}
+  OpenRoad-py.cc


curious if that could be a regular c++ library (i.e: drop the OpenRoad-py.cc dummy file) and have the main binary link against it rather than using OPENROAD_LIB_SRCS?

proppy · 2022-09-27T22:08:51Z

Thanks for working on this @QuantamHD! Made a few comments, let me know if you'd prefer me to do propose the change directly against your branch.

rovinski · 2022-09-28T01:06:55Z

python/pyproject.toml

+authors = [
+    { name = "Matt Liberty", email = "[email protected]"}
+]
+description = "A production ready opensource physical design EDA toolkit"


"production-ready open-source"

Also maybe drop "production-ready" to not convey a bug-free experience :)

rovinski · 2022-09-28T01:08:27Z

python/pyproject.toml

+
+[project]
+name = "openroad"
+version = "0.0.1"


What parses the version number and is this going to break things when it doesn't match the source code version? Can this be generated by cmake?

using a pyproject.toml.in with some @CMAKE_VARIABLES@ or generator expression should be easy
e.g.
A "template" file (here setup.py.in but could be a toml.in file ;))
https://github.com/Mizux/python-native/blob/14648387a267b8ce65db0c0cdd47c1200c8cd7fb/python/setup.py.in#L16-L18
And the CMake stuff to auto-replace...
https://github.com/Mizux/python-native/blob/14648387a267b8ce65db0c0cdd47c1200c8cd7fb/cmake/python.cmake#L173-L181

Last step a custom_command to generate the python package (1 create a the package layout layout with all files in the build dir, 2) ask python to package wheel our stuff, 3) profit !)
https://github.com/Mizux/python-native/blob/14648387a267b8ce65db0c0cdd47c1200c8cd7fb/cmake/python.cmake#L197-L237

maliberty · 2023-03-28T19:02:41Z

What needs to happen next here? This seems stalled

QuantamHD · 2023-03-28T19:07:29Z

We need to complete the librarification of the rest of the openroad modules.

maliberty · 2023-03-28T19:09:57Z

We need to complete the librarification of the rest of the openroad modules.

It seems this could proceed in parallel.

maliberty · 2023-10-25T22:50:59Z

@QuantamHD @proppy any further plans or close?

proppy · 2023-10-26T17:35:44Z

@maliberty I do have interest into landing something like this in the conda package for OpenROAD: https://github.com/hdl/conda-eda/tree/master/pnr/openroad to ease the usage from colab notebooks.

maliberty · 2023-10-26T17:37:32Z

@proppy is there a timeline? As I said earlier "It seems this could proceed in parallel." so I think you are unblocked from progress.

proppy · 2023-10-26T17:43:45Z

Another pattern/reference here that we could reuse/take inspiration from is https://github.com/Mizux/cmake-swig (which I believe is what @Mizux uses for packaging https://github.com/google/or-tools).

maliberty · 2023-10-26T17:50:05Z

@proppy we already have swig setup with cmake. The main point here is about pip packaging

Mizux · 2023-10-26T18:59:04Z

Another pattern/reference here that we could reuse/take inspiration from is https://github.com/Mizux/cmake-swig (which I believe is what @Mizux uses for packaging https://github.com/google/or-tools).

For python only I would look at:

https://github.com/mizux/python-native (C++ + SWIG python)
https://github.com/Mizux/cmake-pybind11 (C++ + pybind11 for wrapping)
https://github.com/Mizux/bazel-pybind11 (Using bazel based build) (also provide cmake based build for comparison)

proppy · 2023-10-26T19:12:41Z

@maliberty but my understanding is that what this PR (despite its title) is trying to address is more building the swig binding as a shared library importable from python #2308 rather than addressing the distribution on pypi #1424 (ultimately we need both, but one need to happen before the other)

proppy · 2023-10-26T19:13:36Z

@Mizux thanks for sharing, that's an awesome reference!

maliberty · 2023-10-26T22:32:02Z

@maliberty but my understanding is that what this PR (despite its title) is trying to address is more building the swig binding as a shared library importable from python #2308 rather than addressing the distribution on pypi #1424 (ultimately we need both, but one need to happen before the other)

There is no question about how to build the python bindings and we've already done a lot of work in that direction and don't plan to change course. The only question is about building shared libraries.

This PR contains python/pyproject.toml which seems related to pip distribution.

If the title is wrong, please fix it.

QuantamHD · 2023-10-26T22:41:13Z

Yeah let me signal boost @maliberty. The bindings part is already solved we have a defined C++ public API that's SWIG'd. The only question is building the shared libraries such that they don't require the giant OpenROAD object that forces you to link the entire app for the library.

@cdleary and I did a bunch of work to separate libraries (gpl, grt, etc.) into library components which should make this easier. There's probably still challenges around logging, and cmake config to be solved.

maliberty · 2023-10-26T22:57:01Z

Thanks @QuantamHD
Even if we have to "link the entire app for the library" it would be better than waiting for the perfect architecture in OR. This is >1year old so something non-ideal is better than nothing at all.

QuantamHD · 2023-10-26T23:05:56Z

There's actually a problem building it with the whole app. It won't link because it's pulling in main symbols which python will reject. The project is currently achieving python by linking a runtime in, and that's incompatible with pip who wants you to link into the user's arbitrary runtime.

maliberty · 2023-10-26T23:07:33Z

There's actually a problem building it with the whole app. It won't link because it's pulling in main symbols which python will reject.

I didn't mean that quite so literally. I just meant making a single .so for everything but Main.cc is tolerable.

QuantamHD · 2023-10-26T23:08:58Z

A single .so is still the goal, it's just a question of actually getting one of those that actually works with pip that's the hard part. I was just pointing out one problem I ran into that stopped progress on this PR. Pulling out Main.cc wasn't as trivial as I expected it to be.

maliberty · 2023-10-26T23:12:25Z

@QuantamHD I'm missing something here. The work you and Chris did was so you could use a single library in near isolation for unit testing. How does that relate to building a single .so? What are the issues with pulling out Main?

QuantamHD · 2023-10-26T23:22:57Z

It's all related @maliberty. I don't recall the exact details, but there's something in Main that the OpenROAD object depends on, and without it none of the logging builds.

proppy · 2023-10-27T00:16:24Z

which seems related to pip distribution

Sorry for the miss-understanding: I do agree all step to get there are inter-related, I just thought it would be beneficial to talk about them in isolation.
a/ writing the swig binding (done)
b/ building a python importable shared library (this PR)
c/ packaging this shared library as a platform wheel that works on most linux distro (a bit of this PR, but more is needed)
d/ automating the distribution on PyPi

maliberty · 2023-10-27T03:38:01Z

It would be helpful if this PR were brought up to date.

Please give a clear statement of the specific issue to be solved is given. All I know is that "something in Main that the OpenROAD object depends on..." but that isn't enough to take any action.

maliberty · 2023-10-27T03:40:43Z

Its also confusing that the title says Draft but this isn't actually a draft PR. Is this seeking help or just in progress?

QuantamHD · 2023-10-27T03:42:55Z

It was draft because it was broken. I don't really have time to work on it at the moment. So this is basically scaffolding that some can pick up, or if no one is interested then it can be closed, and I'll pick it up again when I have more time to focus on this.

QuantamHD mentioned this pull request Sep 27, 2022

package swig bindings as shared libraries #2308

Open

Adds python pip support

cc726e4

Signed-off-by: Ethan Mahintorabi <[email protected]>

QuantamHD force-pushed the python_support branch from 84ea93c to cc726e4 Compare September 27, 2022 18:27

proppy reviewed Sep 27, 2022

View reviewed changes

rovinski reviewed Sep 28, 2022

View reviewed changes

proppy mentioned this pull request Jan 6, 2023

python modules not working in python 3.10 #2535

Closed

maliberty changed the title ~~Draft: Adds python pip support~~ Draft: Build OR shared libraries for PIP distribution Oct 27, 2023

maliberty closed this Nov 29, 2023

maliberty mentioned this pull request Dec 15, 2023

Add Python API #1812

Open

maliberty added the py Python API label Dec 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Build OR shared libraries for PIP distribution #2311

Draft: Build OR shared libraries for PIP distribution #2311

QuantamHD commented Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022

proppy Sep 27, 2022 •

edited

Loading

proppy commented Sep 27, 2022

rovinski Sep 28, 2022

rovinski Sep 28, 2022

Mizux Oct 26, 2023 •

edited

Loading

maliberty commented Mar 28, 2023

QuantamHD commented Mar 28, 2023

maliberty commented Mar 28, 2023

maliberty commented Oct 25, 2023

proppy commented Oct 26, 2023

maliberty commented Oct 26, 2023

proppy commented Oct 26, 2023 •

edited

Loading

maliberty commented Oct 26, 2023

Mizux commented Oct 26, 2023

proppy commented Oct 26, 2023

proppy commented Oct 26, 2023

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 •

edited

Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 •

edited

Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 •

edited

Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 •

edited

Loading

proppy commented Oct 27, 2023

maliberty commented Oct 27, 2023

maliberty commented Oct 27, 2023

QuantamHD commented Oct 27, 2023

		from setuptools.command.build_ext import build_ext as build_ext_orig


		class CMakeExtension(Extension):

Draft: Build OR shared libraries for PIP distribution #2311

Draft: Build OR shared libraries for PIP distribution #2311

Conversation

QuantamHD commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

proppy commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mizux Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

maliberty commented Mar 28, 2023

QuantamHD commented Mar 28, 2023

maliberty commented Mar 28, 2023

maliberty commented Oct 25, 2023

proppy commented Oct 26, 2023

maliberty commented Oct 26, 2023

proppy commented Oct 26, 2023 • edited Loading

maliberty commented Oct 26, 2023

Mizux commented Oct 26, 2023

proppy commented Oct 26, 2023

proppy commented Oct 26, 2023

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 • edited Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 • edited Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 • edited Loading

maliberty commented Oct 26, 2023

QuantamHD commented Oct 26, 2023 • edited Loading

proppy commented Oct 27, 2023

maliberty commented Oct 27, 2023

maliberty commented Oct 27, 2023

QuantamHD commented Oct 27, 2023

proppy Sep 27, 2022 •

edited

Loading

Mizux Oct 26, 2023 •

edited

Loading

proppy commented Oct 26, 2023 •

edited

Loading

QuantamHD commented Oct 26, 2023 •

edited

Loading

QuantamHD commented Oct 26, 2023 •

edited

Loading

QuantamHD commented Oct 26, 2023 •

edited

Loading

QuantamHD commented Oct 26, 2023 •

edited

Loading