-
Notifications
You must be signed in to change notification settings - Fork 15
feat(hugr-py): Allow linking packages and modules from Python #2947
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
Changes from 11 commits
95c77f9
da1482b
4a8a4a9
87cd79f
c519d20
136ce79
58697fc
2a1d7bc
76ada56
15574aa
8ca8496
cb6582e
bf371ab
3a2323c
1d200f3
7b7e790
705bc20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| //! Bindings for linking utilities defined in the hugr-core crate | ||
|
|
||
| use pyo3::exceptions::PyException; | ||
| use pyo3::{create_exception, pymodule}; | ||
|
|
||
| #[pymodule(submodule)] | ||
| #[pyo3(module = "hugr._hugr.linking")] | ||
| pub mod linking { | ||
| use hugr_core::envelope::EnvelopeConfig; | ||
| use hugr_core::hugr::hugrmut::HugrMut; | ||
| use hugr_core::hugr::linking::{HugrLinking, NameLinkingPolicy}; | ||
| use hugr_core::{Hugr, HugrView}; | ||
| use pyo3::exceptions::PyValueError; | ||
| use pyo3::types::{PyAnyMethods, PyModule}; | ||
| use pyo3::{Bound, PyResult, Python, pyfunction}; | ||
|
|
||
| /// Hack: workaround for <https://github.com/PyO3/pyo3/issues/759> | ||
| #[pymodule_init] | ||
| fn init(m: &Bound<'_, PyModule>) -> PyResult<()> { | ||
| Python::attach(|py| { | ||
| py.import("sys")? | ||
| .getattr("modules")? | ||
| .set_item("hugr._hugr.linking", m) | ||
| }) | ||
| } | ||
|
|
||
| #[pyfunction] | ||
| fn link_modules(module_into: &[u8], module_from: &[u8]) -> PyResult<Vec<u8>> { | ||
| let (mut hugr_into, mut exts_into) = | ||
| Hugr::load_with_exts(module_into, None).map_err(|err| { | ||
| PyValueError::new_err(format!("Loading of first envelope failed: {}", err)) | ||
| })?; | ||
| let (hugr_from, exts_from) = Hugr::load_with_exts(module_from, None).map_err(|err| { | ||
| PyValueError::new_err(format!("Loading of second envelope failed: {}", err)) | ||
| })?; | ||
| let into_executable = hugr_into.entrypoint() != hugr_into.module_root(); | ||
| let from_executable = hugr_from.entrypoint() != hugr_from.module_root(); | ||
| let replacement_entrypoint = if into_executable && from_executable { | ||
| return Err(PyValueError::new_err( | ||
| "Cannot link two executable modules together.", | ||
| )); | ||
| } else if !into_executable && from_executable { | ||
| Some(hugr_from.entrypoint()) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let forest = hugr_into | ||
| .link_module(hugr_from, &NameLinkingPolicy::default()) | ||
| .map_err(|err| super::HugrLinkingError::new_err(err.to_string()))?; | ||
| if let Some(new_entrypoint) = replacement_entrypoint { | ||
| let Some(node) = forest.node_map.get(&new_entrypoint) else { | ||
| panic!("Entrypoint is to be replaced but was not found after linking"); | ||
| }; | ||
| hugr_into.set_entrypoint(*node); | ||
| } | ||
| exts_into.extend(exts_from); | ||
|
|
||
| let mut result = Vec::new(); | ||
| hugr_into | ||
| .store_with_exts(&mut result, EnvelopeConfig::binary(), &exts_into) | ||
| .unwrap(); | ||
|
|
||
| hugr_core::package::Package::load(&result[..], None) | ||
|
||
| .map_err(|err| PyValueError::new_err(format!("Roundtrip failed: {:?}", err)))?; | ||
|
|
||
| Ok(result) | ||
| } | ||
| } | ||
|
|
||
| create_exception!( | ||
| _hugr.linking, | ||
| HugrLinkingError, | ||
| PyException, | ||
| "Base exception for HUGR linking errors." | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class HugrLinkingError(Exception): ... | ||
|
|
||
| def link_modules(module_into: bytes, module_from: bytes) -> bytes: ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import pytest | ||
|
|
||
| from hugr import Hugr, tys | ||
| from hugr._hugr.linking import link_modules | ||
| from hugr.build import Module | ||
| from hugr.ops import FuncDefn | ||
| from hugr.package import Package | ||
| from hugr.std import float, int, logic, ptr | ||
|
|
||
|
|
||
| def build_module(*, entrypoint: bool) -> Hugr: | ||
| builder = Module() | ||
| if entrypoint: | ||
| main = builder.define_function( | ||
| "main", input_types=[tys.Bool], output_types=[tys.Bool], visibility="Public" | ||
| ) | ||
| main.set_outputs(*main.inputs()) | ||
| builder.hugr.entrypoint = main.parent_node | ||
|
|
||
| return builder.hugr | ||
|
|
||
|
|
||
| def test_link_modules_no_entrypoints(): | ||
| hugr1 = build_module(entrypoint=False) | ||
| hugr2 = build_module(entrypoint=False) | ||
|
|
||
| linked = Hugr.from_bytes(link_modules(hugr1.to_bytes(), hugr2.to_bytes())) | ||
| assert linked.entrypoint == linked.module_root | ||
|
|
||
|
|
||
| def test_link_modules_entrypoint_lhs(): | ||
| hugr1 = build_module(entrypoint=True) | ||
| hugr2 = build_module(entrypoint=False) | ||
|
|
||
| linked = Hugr.from_bytes(link_modules(hugr1.to_bytes(), hugr2.to_bytes())) | ||
| assert linked.entrypoint != linked.module_root | ||
| entrypoint = linked.entrypoint_op() | ||
| assert isinstance(entrypoint, FuncDefn) | ||
| assert entrypoint.f_name == "main" | ||
|
|
||
|
|
||
| def test_link_modules_entrypoint_rhs(): | ||
| hugr1 = build_module(entrypoint=False) | ||
| hugr2 = build_module(entrypoint=True) | ||
|
|
||
| linked = Hugr.from_bytes(link_modules(hugr1.to_bytes(), hugr2.to_bytes())) | ||
| assert linked.entrypoint != linked.module_root | ||
| entrypoint = linked.entrypoint_op() | ||
| assert isinstance(entrypoint, FuncDefn) | ||
| assert entrypoint.f_name == "main" | ||
|
|
||
|
|
||
| def test_link_modules_multiple_entrypoints(): | ||
| hugr1 = build_module(entrypoint=True) | ||
| hugr2 = build_module(entrypoint=True) | ||
|
|
||
| with pytest.raises(ValueError, match="Cannot link two executable modules together"): | ||
| link_modules(hugr1.to_bytes(), hugr2.to_bytes()) | ||
|
|
||
|
|
||
| def test_link_packages_no_modules(): | ||
| pkg1 = Package(modules=[]) | ||
| pkg2 = Package(modules=[]) | ||
|
|
||
| result_pkg = pkg1.link(pkg2) | ||
|
|
||
| assert result_pkg.modules == [] | ||
|
|
||
|
|
||
| def test_link_packages_extensions(): | ||
| pkg1 = Package( | ||
| modules=[build_module(entrypoint=False)], | ||
| extensions=[ | ||
| int.CONVERSIONS_EXTENSION, | ||
| int.INT_TYPES_EXTENSION, | ||
| int.INT_OPS_EXTENSION, | ||
| # Shared | ||
| logic.EXTENSION, | ||
| ptr.EXTENSION, | ||
| ], | ||
| ) | ||
| pkg2 = Package( | ||
| modules=[build_module(entrypoint=False)], | ||
| extensions=[ | ||
| float.FLOAT_OPS_EXTENSION, | ||
| float.FLOAT_TYPES_EXTENSION, | ||
| # Shared | ||
| logic.EXTENSION, | ||
| ptr.EXTENSION, | ||
| ], | ||
| ) | ||
|
|
||
| result_pkg = pkg1.link(pkg2) | ||
|
|
||
| assert result_pkg.extensions == [ | ||
| int.CONVERSIONS_EXTENSION, | ||
| int.INT_TYPES_EXTENSION, | ||
| int.INT_OPS_EXTENSION, | ||
| logic.EXTENSION, | ||
| ptr.EXTENSION, | ||
| float.FLOAT_OPS_EXTENSION, | ||
| float.FLOAT_TYPES_EXTENSION, | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that we're going to have all three hugrs in memory, which seems like it could be a problem.
I don't really have strong enough rust/pyO3 foo to say the solution, but possibly:
impl Readerso the files are read here and their contents can be droppedPackageas argument so we can consume them when they're linked in and release the memoryThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could instead take an owned
Vec<u8>if that helps... See https://pyo3.rs/v0.28.2/conversions/tables.html for allowed types / types that have an implementation of the required traits out of the box.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that'd be good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified the param. It seems that the reader trait to read into a hugr is not implemented for a vec though, so I still use a slice. It might also not make much of a difference since Python has no concept of ownership, so I presume the contents are copied anyway. This might actually be more expensive now 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I benchmarked it, and using the QAOA example using
Vec<u8I had about0.048sper link call (from Python), whereas using&[u8]gave me about0.031sper link call (probably due to avoiding an unnecessary copy. I reverted the change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's fair 👍