[DAGCircuit Oxidation] Port DAGNode to Rust
#12380
Conversation
Also has beginnings of DAGCircuit, but this is not complete or wired up at all.
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 9068130816Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Overall this looks good to me, thanks for doing it! I mostly had comments about our pickle handling through these cases - I think there's a fair amount of places where we could entirely elide the __getstate__ and __setstate__ calls and do it all through __getnewargs__ (and some places where we're duplicating things already).
The other super minor point is whether you think it's worth deferring to Python-space formatting in __repr__ methods. It's not as fast, but if anything is bottlenecked on __repr__ we should probably make sure it isn't, and using the Python-space formatting means everything will be consistent no matter how deep or shallow in the object hierarchy a __repr__ is called.
There was a problem hiding this comment.
This looks fine to merge to me, thanks for the changes.
Just for visibility for anyone else looking: the three-tuple return from __reduce__ here where the callable is the Self type object can also be written as a __getnewargs__/__getstate__/__setstate__ triple, but the current way also works totally fine.
| fn __getstate__(&self) -> isize { | ||
| self._node_id | ||
| } | ||
|
|
||
| fn __setstate__(&mut self, nid: isize) { | ||
| self._node_id = nid; | ||
| } |
There was a problem hiding this comment.
Strictly these two could also be replaced with a __getnewargs__ as well, but it's unimportant - I wouldn't be surprised if they're never called at all, since I don't think we ever construct bare DAGNode.
jakelishman
added
Rust
Summary
Ports
DAGNodeand subclasses to Rust. This is precursory to writing efficient ports of methods onDAGCircuitin Rust, since we can avoid a lot of interpreter interaction.Details and comments
Everything is ported over with the exception of
semantic_eq, since its implementation has a lot of interplay with components that we currently must access through the Python interpreter (e.g.Expr,Parameter, registers, etc.).