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.

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

Replace HugrInternals::base_hugr with simpler methods #1926

Open
aborgna-q opened this issue Feb 24, 2025 · 0 comments
Open

Replace HugrInternals::base_hugr with simpler methods #1926

aborgna-q opened this issue Feb 24, 2025 · 0 comments

Comments

@aborgna-q
Copy link
Collaborator

HugrInternals::base_hugr returns the root Hugr used to implement a hugr view.
In practice, this call is used in a handful of cases:

  • To access the metadata/optype for a single node.
  • To access the ExtensionRegistry.
  • To access the Hierarchy that linked to the HugrInternals::portgraph
  • To run full-graph validation
  • To render mermaid/dot of the full base graph
  • In SiblingGraph, to avoid nesting views when creating one from a generic HugrView.

This works fine for our current definitions, but non-trivial wrappers like the one discussed in CQCL/tket2#778 are not necessarily based on a single base Hugr.

All but the last point I listed can be implemented as individual methods on HugrInternals instead,

  • metadata(&self, Node) -> Option<NodeMetadataMap>
  • optype(&self, None) -> Option<OpType>
  • extension_registry(&self) -> ExtensionRegistry
  • hierarchy would return a Cow<'_, Hierarchy> so we can either return a reference to the internally stored one, or compute one on-the-fly (for the CircuitHistory usecase).
  • Rendering should use the existing portgraph directly.
  • SiblingGraph shouldn't need to do the unwrapping. For non-dyn types all the calls through wrapping layers will hopefully be inlined and optimized away (we should benchmark this).

One drawback to keep in mind is that we no longer get the generic "escape hatch" that lets us recover the outer hugr from a filtered view. This means for example that focusing on a region will hide away non-local nodes outside the parent's hierarchy. I'd argue that those cases should use region-pointers instead. See #1642 #1554.

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

No branches or pull requests

1 participant