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.

Sign up for GitHub

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

Jump to bottom

Rust Integration: Full Rust Integration #2

Closed
wants to merge 32 commits into from
Closed

Rust Integration: Full Rust Integration #2

wants to merge 32 commits into from

Conversation

ohjuny
Copy link
Collaborator

@ohjuny ohjuny commented Nov 7, 2024 •
edited
Loading

This PR follows #1.
#1: Sets up pyo3 integration and replaces calls to nx.maximum_flow with a Rust implementation. In particular, #1 constructs and destructs a Rust-side PhillipsDessouky object every iteration, and therefore incurs interop overhead at every iteration.

This PR seeks to replace all (or most of) solver.py such that all the iterations can be performed in Rust without having to go back to Python.

TODO: more information

ohjuny and others added 30 commits October 18, 2024 17:06
@ohjuny
set up PR for naive implementation that copies each iteration
e85d6f8
@ohjuny
set up py03 directories
9bd30b8
@ohjuny
add rust dependencies
5af0b31
@ohjuny
add networkx dependency for testing
67979ae
@ohjuny
rust-side PhillipsDessouky
e4f9b3f
@ohjuny
python-side profiling code
743336f
@ohjuny
integrate rust-side library with python solver
43f0832
@ohjuny
black formatting
4048d9f
@ohjuny
remove unnecessary dependency
463dd20
@ohjuny
factor out rust interop pre/post-processing
5a7d108
@ohjuny
replace second max_flow call with rust
3e07341
@ohjuny
black formatting
556940a
@ohjuny
readd commented old max flow code for potential future where we provi…
9041daf 
…de option to run rust or python versions
@ohjuny
fix refactoring, need to double check with run
4acad7b
@ohjuny
comment second max flow (issue with replacing second, will find out)
af5897c
@ohjuny
remove rust-side commented i64 code
cd5c40e
@ohjuny @jaywonchung
rust-side cleanup
45dabdb 
Co-authored-by: Jae-Won Chung <[email protected]>
@ohjuny
implement 2nd max flow + attempt pimpl for rust-side. note that using…
93a1a78 
… pimpl version is actually slower
@ohjuny
remove unused pimpl implementation
1f37443
@ohjuny
fix lint errors + rust comments
dbb90d9
@ohjuny
rename lowtime_rust to lowtime_rs
2c5c5bc
@ohjuny
figuring out how to package rust/pyo3 module within python distribution
7b14211
@ohjuny
restructure as single package using maturin
87a204a
@ohjuny
fix nodeview type annotations
10cc0c7
@ohjuny
pyi for pyo3 python module
8ae5b7b
@ohjuny
py.typed for pyright check
54e97f3
@ohjuny
fix pyi for _lowtime_rs
a4bb48d
@ohjuny
fix type hint
e4aff75
@ohjuny
remove unnecessary comment
b4a814c
@ohjuny
refactor class impl out of lib
f945bff
@ohjuny ohjuny marked this pull request as draft November 7, 2024 18:50
@ohjuny ohjuny changed the title Lowtime rust full graph Rust Integration: Full Rust Integration Nov 7, 2024
@ohjuny
Copy link
Collaborator Author

ohjuny commented Nov 7, 2024

Note to self: I think I've set up this PR wrong, have never tried making a PR based on another (pending) PR. Will have to figure this out and/or create a new PR and reproduce the changes here in the future.

@jaywonchung
Copy link
Member

Now that the lowtime-rust branch has been updated, you might be better off cutting a new branch from the up-to-date lowtime-rust branch and creating a PR from that new branch.

@ohjuny ohjuny mentioned this pull request Nov 9, 2024
Draft
@ohjuny
Copy link
Collaborator Author

ohjuny commented Nov 9, 2024

Replacing this with #3
This PR was set up with a weird branch structure, so we are replacing it with a cleaner version.

@ohjuny ohjuny closed this Nov 9, 2024
@ohjuny ohjuny deleted the lowtime-rust-full-graph branch November 9, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ohjuny @jaywonchung