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

feat!: HugrView API improvements #680

Merged
merged 35 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
cd5e544
refactor!: move non_df_count to OpTrait
ss2165 Nov 11, 2023
2b1bd37
fix: builder `call` port calculation incorrect
ss2165 Nov 11, 2023
b959e16
refactor!: remove `static_input`
ss2165 Nov 11, 2023
29350ee
refactor!: clearer OpType port/kind method names
ss2165 Nov 11, 2023
7477cea
feat: iterator over all ports connected to node
ss2165 Nov 11, 2023
e6cc8e1
feat: `static_source` for getting static source node
ss2165 Nov 11, 2023
11ef7ba
refactor!: rename `FunctionType::get` -> `FunctionType::port_type`
ss2165 Nov 13, 2023
0437efc
feat: add `in/out_port_type` to `FunctionType`
ss2165 Nov 13, 2023
47d4646
feat: `HugrView::signature(Node)` and port+type iterators
ss2165 Nov 13, 2023
2ffd29a
feat: single_source/target for when you know only one connected port
ss2165 Nov 13, 2023
da9aea8
feat: `static_targets` and `OpType::static_output_port`
ss2165 Nov 13, 2023
b2a2ae0
feat: `dataflow_ports_only` function to filter node-ports
ss2165 Nov 13, 2023
b4db711
refactor!: OpType::signature returns option (non-dataflow ops don't r…
ss2165 Nov 14, 2023
39e1490
refactor!: OpType::signature returns option (non-dataflow ops don't r…
ss2165 Nov 14, 2023
89e1f26
Merge branch 'temp' into feat/more-hugrview
ss2165 Nov 14, 2023
b1db6cc
refactor: impl DataFlowOpTrait for LeafOp and use dataflow_signature
ss2165 Nov 14, 2023
1677014
feat: use macro for OpType reference casting to inner
ss2165 Nov 14, 2023
5fd1035
refactor: use enum to simplify optype pattern matches
ss2165 Nov 14, 2023
09844a1
chore: remove paste from dev-dependencies
ss2165 Nov 14, 2023
b124160
feat: `all_linked_ports`
ss2165 Nov 14, 2023
328ce45
address minor review comments
ss2165 Nov 14, 2023
44cb5b0
feat!: single_linked_port, single_linked_input, single_linked_output
ss2165 Nov 14, 2023
f7bfb46
refactor!: allow chaining with `dataflow_ports_only`
ss2165 Nov 14, 2023
920880e
feat!: parametric all_linked_ports
ss2165 Nov 14, 2023
2d1e47c
refactor: simpler static_input_port
ss2165 Nov 14, 2023
7ea609a
refactor: remove uncessary trait impls for ExternalOp
ss2165 Nov 14, 2023
37ceea8
simplify static i/o tags
ss2165 Nov 14, 2023
d31794d
refactor: nicer code in `signature.rs`
ss2165 Nov 14, 2023
4636bf7
refactor!: swap `signature` and `dataflow_signature`
ss2165 Nov 14, 2023
0a6bb8a
refactor!: `non_df_port_count` just returns usize
ss2165 Nov 14, 2023
9556156
Merge branch 'main' into feat/more-hugrview
ss2165 Nov 14, 2023
f091b5a
fix: put correct compile flags in
ss2165 Nov 14, 2023
451ce23
fix doclinks
ss2165 Nov 14, 2023
c1822db
Merge branch 'main' into feat/more-hugrview
ss2165 Nov 15, 2023
65ed37a
Merge branch 'main' into feat/more-hugrview
ss2165 Nov 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ petgraph = { version = "0.6.3", default-features = false }
context-iterators = "0.2.0"
serde_json = "1.0.97"
delegate = "0.10.0"
rustversion = "1.0.14"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spicy nightly crimes...

This is alright for now, but we should not publish the crate until 1.75 is stable, and this can be removed.


[features]
pyo3 = ["dep:pyo3"]
Expand Down
15 changes: 8 additions & 7 deletions src/builder/build_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ pub trait Dataflow: Container {
hugr: Hugr,
input_wires: impl IntoIterator<Item = Wire>,
) -> Result<BuildHandle<DataflowOpID>, BuildError> {
let num_outputs = hugr.get_optype(hugr.root()).signature().output_count();
let num_outputs = hugr.get_optype(hugr.root()).value_output_count();
let node = self.add_hugr(hugr)?.new_root;

let inputs = input_wires.into_iter().collect();
Expand All @@ -257,8 +257,8 @@ pub trait Dataflow: Container {
hugr: &impl HugrView,
input_wires: impl IntoIterator<Item = Wire>,
) -> Result<BuildHandle<DataflowOpID>, BuildError> {
let num_outputs = hugr.get_optype(hugr.root()).signature().output_count();
let node = self.add_hugr_view(hugr)?.new_root;
let num_outputs = hugr.get_optype(hugr.root()).value_output_count();

let inputs = input_wires.into_iter().collect();
wire_up_inputs(inputs, node, self)?;
Expand Down Expand Up @@ -612,8 +612,9 @@ pub trait Dataflow: Container {
})
}
};
let const_in_port = signature.output.len();
let op_id = self.add_dataflow_op(ops::Call { signature }, input_wires)?;
let op: OpType = ops::Call { signature }.into();
let const_in_port = op.static_input_port().unwrap();
let op_id = self.add_dataflow_op(op, input_wires)?;
let src_port = self.hugr_mut().num_outputs(function.node()) - 1;

self.hugr_mut()
Expand All @@ -633,13 +634,13 @@ fn add_node_with_wires<T: Dataflow + ?Sized>(
nodetype: impl Into<NodeType>,
inputs: Vec<Wire>,
) -> Result<(Node, usize), BuildError> {
let nodetype = nodetype.into();
let sig = nodetype.op_signature();
let nodetype: NodeType = nodetype.into();
let num_outputs = nodetype.op().value_output_count();
let op_node = data_builder.add_child_node(nodetype)?;

wire_up_inputs(inputs, op_node, data_builder)?;

Ok((op_node, sig.output().len()))
Ok((op_node, num_outputs))
}

fn wire_up_inputs<T: Dataflow + ?Sized>(
Expand Down
5 changes: 3 additions & 2 deletions src/builder/conditional.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::extension::ExtensionRegistry;
use crate::hugr::views::HugrView;
use crate::ops::dataflow::DataflowOpTrait;
use crate::types::{FunctionType, TypeRow};

use crate::ops;
use crate::ops::handle::CaseID;
use crate::ops::{self, OpTrait};

use super::build_traits::SubContainer;
use super::handle::BuildHandle;
Expand Down Expand Up @@ -104,12 +105,12 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> ConditionalBuilder<B> {
pub fn case_builder(&mut self, case: usize) -> Result<CaseBuilder<&mut Hugr>, BuildError> {
let conditional = self.conditional_node;
let control_op = self.hugr().get_optype(self.conditional_node);
let extension_delta = control_op.signature().extension_reqs;

let cond: ops::Conditional = control_op
.clone()
.try_into()
.expect("Parent node does not have Conditional optype.");
let extension_delta = cond.dataflow_signature().extension_reqs;
let inputs = cond
.case_input_row(case)
.ok_or(ConditionalBuildError::NotCase { conditional, case })?;
Expand Down
6 changes: 3 additions & 3 deletions src/builder/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,14 @@ mod test {

let mut f_build = module_builder.define_function(
"main",
FunctionType::new(type_row![NAT], type_row![NAT]).pure(),
FunctionType::new(type_row![NAT], type_row![NAT, NAT]).pure(),
)?;
let local_build = f_build.define_function(
"local",
FunctionType::new(type_row![NAT], type_row![NAT]).pure(),
FunctionType::new(type_row![NAT], type_row![NAT, NAT]).pure(),
)?;
let [wire] = local_build.input_wires_arr();
let f_id = local_build.finish_with_outputs([wire])?;
let f_id = local_build.finish_with_outputs([wire, wire])?;

let call = f_build.call(f_id.handle(), f_build.input_wires())?;

Expand Down
14 changes: 9 additions & 5 deletions src/extension/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,15 @@ impl UnificationContext {
match node_type.signature() {
// Input extensions are open
None => {
let delta = node_type.op_signature().extension_reqs;
let c = if delta.is_empty() {
Constraint::Equal(m_input)
let c = if let Some(sig) = node_type.op_signature() {
let delta = sig.extension_reqs;
if delta.is_empty() {
Constraint::Equal(m_input)
} else {
Constraint::Plus(delta, m_input)
}
} else {
Constraint::Plus(delta, m_input)
Constraint::Equal(m_input)
};
self.add_constraint(m_output, c);
}
Expand Down Expand Up @@ -1063,7 +1067,7 @@ mod test {
hugr,
conditional_node,
op.clone(),
Into::<OpType>::into(op).signature(),
Into::<OpType>::into(op).signature().unwrap(),
)?;

let lift1 = hugr.add_node_with_parent(
Expand Down
9 changes: 7 additions & 2 deletions src/extension/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ impl ExtensionValidator {
pub fn new(hugr: &Hugr, closure: ExtensionSolution) -> Self {
let mut extensions: HashMap<(Node, Direction), ExtensionSet> = HashMap::new();
for (node, incoming_sol) in closure.into_iter() {
let op_signature = hugr.get_nodetype(node).op_signature();
let outgoing_sol = op_signature.extension_reqs.union(&incoming_sol);
let extension_reqs = hugr
.get_nodetype(node)
.op_signature()
.map(|s| s.extension_reqs)
.unwrap_or_default();

let outgoing_sol = extension_reqs.union(&incoming_sol);

extensions.insert((node, Direction::Incoming), incoming_sol);
extensions.insert((node, Direction::Outgoing), outgoing_sol);
Expand Down
11 changes: 7 additions & 4 deletions src/hugr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,16 @@ impl NodeType {

/// Use the input extensions to calculate the concrete signature of the node
pub fn signature(&self) -> Option<Signature> {
self.input_extensions
.as_ref()
.map(|rs| self.op.signature().with_input_extensions(rs.clone()))
self.input_extensions.as_ref().map(|rs| {
self.op
.signature()
.unwrap_or_default()
.with_input_extensions(rs.clone())
})
}

/// Get the function type from the embedded op
pub fn op_signature(&self) -> FunctionType {
pub fn op_signature(&self) -> Option<FunctionType> {
self.op.signature()
}

Expand Down
4 changes: 2 additions & 2 deletions src/hugr/hugrmut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,12 @@ impl<T: RootTagged<RootHandle = Node> + AsMut<Hugr>> HugrMut for T {
) -> Result<(OutgoingPort, IncomingPort), HugrError> {
let src_port = self
.get_optype(src)
.other_port_index(Direction::Outgoing)
.other_output_port()
.expect("Source operation has no non-dataflow outgoing edges")
.as_outgoing()?;
let dst_port = self
.get_optype(dst)
.other_port_index(Direction::Incoming)
.other_input_port()
.expect("Destination operation has no non-dataflow incoming edges")
.as_incoming()?;
self.connect(src, src_port, dst, dst_port)?;
Expand Down
5 changes: 1 addition & 4 deletions src/hugr/rewrite/insert_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::{HugrView, IncomingPort};

use super::Rewrite;

use itertools::Itertools;
use thiserror::Error;

/// Specification of a identity-insertion operation.
Expand Down Expand Up @@ -73,9 +72,7 @@ impl Rewrite for IdentityInsertion {
};

let (pre_node, pre_port) = h
.linked_outputs(self.post_node, self.post_port)
.exactly_one()
.ok()
.single_source(self.post_node, self.post_port)
.expect("Value kind input can only have one connection.");

h.disconnect(self.post_node, self.post_port).unwrap();
Expand Down
5 changes: 3 additions & 2 deletions src/hugr/rewrite/outline_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ impl OutlineCfg {
}
}
}
extension_delta = extension_delta.union(&o.signature().extension_reqs);
extension_delta = extension_delta
.union(&o.signature().expect("cfg missing signature").extension_reqs);
let external_succs = h.output_neighbours(n).filter(|s| !self.blocks.contains(s));
match external_succs.at_most_one() {
Ok(None) => (), // No external successors
Expand Down Expand Up @@ -177,7 +178,7 @@ impl Rewrite for OutlineCfg {
let exit_port = h
.node_outputs(exit)
.filter(|p| {
let (t, p2) = h.linked_ports(exit, *p).exactly_one().ok().unwrap();
let (t, p2) = h.single_target(exit, *p).unwrap();
assert!(p2.index() == 0);
t == outside
})
Expand Down
7 changes: 3 additions & 4 deletions src/hugr/rewrite/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,8 @@ impl NewEdgeSpec {
true
};
let found_incoming = h
.linked_ports(self.tgt, tgt_pos)
.exactly_one()
.is_ok_and(|(src_n, _)| descends_from_legal(src_n));
.single_source(self.tgt, tgt_pos)
.is_some_and(|(src_n, _)| descends_from_legal(src_n));
if !found_incoming {
return Err(ReplaceError::NoRemovedEdge(err_edge()));
};
Expand Down Expand Up @@ -617,7 +616,7 @@ mod test {
entry: bool,
) -> Result<BasicBlockID, BuildError> {
let op: OpType = op.into();
let op_sig = op.signature();
let op_sig = op.signature().expect("dataflow op needs signature");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm, we can't do something like impl Into<DataflowOpTrait> can we? Then we'd know we have a signature. I guess nothing to worry about in a test but generally it feels like it'd be good to try to know statically when we have an Op(Type) with a signature

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, I've used this logic in ~3 places, there are probably more. (Could be a follow up task for builder to require DataflowOpTrait in more places rather than Into<OpType>?)

let mut bb = if entry {
assert_eq!(
match h.hugr().get_optype(h.container_node()) {
Expand Down
45 changes: 22 additions & 23 deletions src/hugr/rewrite/simple_replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ use std::collections::{hash_map, HashMap};
use std::iter::{self, Copied};
use std::slice;

use itertools::Itertools;

use crate::hugr::views::SiblingSubgraph;
use crate::hugr::{HugrMut, HugrView, NodeMetadataMap, Rewrite};
use crate::ops::{OpTag, OpTrait, OpType};
Expand Down Expand Up @@ -129,11 +127,8 @@ impl Rewrite for SimpleReplacement {
for ((rep_inp_node, rep_inp_port), (rem_inp_node, rem_inp_port)) in &self.nu_inp {
if self.replacement.get_optype(*rep_inp_node).tag() != OpTag::Output {
// add edge from predecessor of (s_inp_node, s_inp_port) to (new_inp_node, n_inp_port)
let (rem_inp_pred_node, rem_inp_pred_port) = h
.linked_outputs(*rem_inp_node, *rem_inp_port)
.exactly_one()
.ok() // PortLinks does not implement Debug
.unwrap();
let (rem_inp_pred_node, rem_inp_pred_port) =
h.single_source(*rem_inp_node, *rem_inp_port).unwrap();
h.disconnect(*rem_inp_node, *rem_inp_port).unwrap();
let new_inp_node = index_map.get(rep_inp_node).unwrap();
h.connect(
Expand All @@ -150,8 +145,7 @@ impl Rewrite for SimpleReplacement {
for ((rem_out_node, rem_out_port), rep_out_port) in &self.nu_out {
let (rep_out_pred_node, rep_out_pred_port) = self
.replacement
.linked_outputs(replacement_output_node, *rep_out_port)
.exactly_one()
.single_source(replacement_output_node, *rep_out_port)
.unwrap();
if self.replacement.get_optype(rep_out_pred_node).tag() != OpTag::Input {
let new_out_node = index_map.get(&rep_out_pred_node).unwrap();
Expand All @@ -171,11 +165,8 @@ impl Rewrite for SimpleReplacement {
let rem_inp_nodeport = self.nu_inp.get(&(replacement_output_node, rep_out_port));
if let Some((rem_inp_node, rem_inp_port)) = rem_inp_nodeport {
// add edge from predecessor of (rem_inp_node, rem_inp_port) to (rem_out_node, rem_out_port):
let (rem_inp_pred_node, rem_inp_pred_port) = h
.linked_outputs(*rem_inp_node, *rem_inp_port)
.exactly_one()
.ok() // PortLinks does not implement Debug
.unwrap();
let (rem_inp_pred_node, rem_inp_pred_port) =
h.single_source(*rem_inp_node, *rem_inp_port).unwrap();
h.disconnect(*rem_inp_node, *rem_inp_port).unwrap();
h.disconnect(*rem_out_node, *rem_out_port).unwrap();
h.connect(
Expand Down Expand Up @@ -505,15 +496,27 @@ pub(in crate::hugr::rewrite) mod test {
.collect_vec();
let inputs = h
.node_outputs(input)
.filter(|&p| h.get_optype(input).signature().get(p).is_some())
.filter(|&p| {
h.get_optype(input)
.signature()
.unwrap()
.port_type(p)
.is_some()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps an OpType::df_port_type helper would be useful?

Alternatively,

Suggested change
h.get_optype(input)
.signature()
.unwrap()
.port_type(p)
.is_some()
h.signature(input).is_some_and(|s| s.port_type(p).is_some())

(there's many other cases of hugr.get_optype(n).signature()).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken a different approach - casting to known types and using
dataflow_signature rather than continuing to wrap FunctionType methods on to OpType

})
.map(|p| {
let link = h.linked_inputs(input, p).next().unwrap();
(link, link)
})
.collect();
let outputs = h
.node_inputs(output)
.filter(|&p| h.get_optype(output).signature().get(p).is_some())
.filter(|&p| {
h.get_optype(output)
.signature()
.unwrap()
.port_type(p)
.is_some()
})
.map(|p| ((output, p), p))
.collect();
h.apply_rewrite(SimpleReplacement::new(
Expand Down Expand Up @@ -565,7 +568,7 @@ pub(in crate::hugr::rewrite) mod test {

let outputs = repl
.node_inputs(repl_output)
.filter(|&p| repl.get_optype(repl_output).signature().get(p).is_some())
.filter(|&p| repl.signature(repl_output).unwrap().port_type(p).is_some())
.map(|p| ((repl_output, p), p))
.collect();

Expand Down Expand Up @@ -598,7 +601,7 @@ pub(in crate::hugr::rewrite) mod test {
if *tgt == out {
unimplemented!()
};
let (src, src_port) = h.linked_outputs(*r_n, *r_p).exactly_one().ok().unwrap();
let (src, src_port) = h.single_source(*r_n, *r_p).unwrap();
NewEdgeSpec {
src,
tgt: *tgt,
Expand All @@ -613,11 +616,7 @@ pub(in crate::hugr::rewrite) mod test {
.nu_out
.iter()
.map(|((tgt, tgt_port), out_port)| {
let (src, src_port) = replacement
.linked_outputs(out, *out_port)
.exactly_one()
.ok()
.unwrap();
let (src, src_port) = replacement.single_source(out, *out_port).unwrap();
if src == in_ {
unimplemented!()
};
Expand Down
5 changes: 2 additions & 3 deletions src/hugr/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use pyo3::{create_exception, exceptions::PyException, PyErr};
use crate::core::NodeIndex;
use crate::extension::ExtensionSet;
use crate::hugr::{Hugr, NodeType};
use crate::ops::OpTrait;
use crate::ops::OpType;
use crate::{Node, PortIndex};
use portgraph::hierarchy::AttachError;
Expand Down Expand Up @@ -167,7 +166,7 @@ impl TryFrom<&Hugr> for SerHugrV0 {
.expect("Could not reach one of the nodes");

let find_offset = |node: Node, offset: usize, dir: Direction, hugr: &Hugr| {
let sig = hugr.get_optype(node).signature();
let sig = hugr.signature(node).unwrap_or_default();
let offset = match offset < sig.port_count(dir) {
true => Some(offset as u16),
false => None,
Expand Down Expand Up @@ -246,7 +245,7 @@ impl TryFrom<SerHugrV0> for Hugr {
None => {
let op_type = hugr.get_optype(node);
op_type
.other_port_index(dir)
.other_port(dir)
.ok_or(HUGRSerializationError::MissingPortOffset {
node,
op_type: op_type.clone(),
Expand Down
2 changes: 2 additions & 0 deletions src/hugr/validate/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,10 @@ fn test_local_const() -> Result<(), HugrError> {
// Second input of Xor from a constant
let cst = h.add_node_with_parent(h.root(), const_op)?;
let lcst = h.add_node_with_parent(h.root(), ops::LoadConstant { datatype: BOOL_T })?;

h.connect(cst, 0, lcst, 0)?;
h.connect(lcst, 0, and, 1)?;
assert_eq!(h.static_source(lcst), Some(cst));
// There is no edge from Input to LoadConstant, but that's OK:
h.update_validate(&EMPTY_REG).unwrap();
Ok(())
Expand Down
Loading