From 95e40ad3aded2b2b793257487403520a24c9719a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 10:59:02 +0000 Subject: [PATCH 01/18] Combine ExtensionSolutions (no separate closure) --- quantinuum-hugr/src/extension/infer.rs | 12 ++--------- quantinuum-hugr/src/extension/infer/test.rs | 13 ++++++------ quantinuum-hugr/src/hugr.rs | 22 ++++++++++----------- quantinuum-hugr/src/hugr/validate/test.rs | 12 +++++------ 4 files changed, 24 insertions(+), 35 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer.rs b/quantinuum-hugr/src/extension/infer.rs index 64fca0265..29d9db74a 100644 --- a/quantinuum-hugr/src/extension/infer.rs +++ b/quantinuum-hugr/src/extension/infer.rs @@ -37,18 +37,10 @@ pub type ExtensionSolution = HashMap; /// closure: a solution which would be valid if all of the variables in the graph /// were instantiated to an empty extension set. This is used (by validation) to /// concretise the extension requirements of the whole hugr. -pub fn infer_extensions( - hugr: &impl HugrView, -) -> Result<(ExtensionSolution, ExtensionSolution), InferExtensionError> { +pub fn infer_extensions(hugr: &impl HugrView) -> Result { let mut ctx = UnificationContext::new(hugr); - let solution = ctx.main_loop()?; ctx.instantiate_variables(); - let closed_solution = ctx.main_loop()?; - let closure: ExtensionSolution = closed_solution - .into_iter() - .filter(|(node, _)| !solution.contains_key(node)) - .collect(); - Ok((solution, closure)) + ctx.main_loop() } /// Metavariables don't need much diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 91aa7788b..87b018f50 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -100,13 +100,13 @@ fn from_graph() -> Result<(), Box> { hugr.connect(mult_c, 0, output, 0); - let (_, closure) = infer_extensions(&hugr)?; + let solution = infer_extensions(&hugr)?; let empty = ExtensionSet::new(); let ab = ExtensionSet::from_iter([A, B]); - assert_eq!(*closure.get(&(hugr.root())).unwrap(), empty); - assert_eq!(*closure.get(&(mult_c)).unwrap(), ab); - assert_eq!(*closure.get(&(add_ab)).unwrap(), empty); - assert_eq!(*closure.get(&add_b).unwrap(), ExtensionSet::singleton(&A)); + assert_eq!(*solution.get(&(hugr.root())).unwrap(), empty); + assert_eq!(*solution.get(&(mult_c)).unwrap(), ab); + assert_eq!(*solution.get(&(add_ab)).unwrap(), empty); + assert_eq!(*solution.get(&add_b).unwrap(), ExtensionSet::singleton(&A)); Ok(()) } @@ -249,8 +249,7 @@ fn dangling_src() -> Result<(), Box> { hugr.connect(src, 0, mult, 1); hugr.connect(mult, 0, output, 0); - let closure = hugr.infer_extensions()?; - assert!(closure.is_empty()); + hugr.infer_extensions()?; assert_eq!(hugr.get_nodetype(src.node()).io_extensions().unwrap().1, rs); assert_eq!( hugr.get_nodetype(mult.node()).io_extensions().unwrap(), diff --git a/quantinuum-hugr/src/hugr.rs b/quantinuum-hugr/src/hugr.rs index c92231804..1d5bba5ec 100644 --- a/quantinuum-hugr/src/hugr.rs +++ b/quantinuum-hugr/src/hugr.rs @@ -198,24 +198,22 @@ impl Hugr { extension_registry: &ExtensionRegistry, ) -> Result<(), ValidationError> { resolve_extension_ops(self, extension_registry)?; - let closure = self.infer_extensions()?; - self.validate_with_extension_closure(closure, extension_registry)?; + self.infer_extensions()?; + self.validate(extension_registry)?; Ok(()) } /// Infer extension requirements and add new information to `op_types` field + /// (if the "extension_inference" feature is on; otherwise, do nothing) /// /// See [`infer_extensions`] for details on the "closure" value - #[cfg(feature = "extension_inference")] - pub fn infer_extensions(&mut self) -> Result { - let (solution, extension_closure) = infer_extensions(self)?; - self.instantiate_extensions(solution); - Ok(extension_closure) - } - /// Do nothing - this functionality is gated by the feature "extension_inference" - #[cfg(not(feature = "extension_inference"))] - pub fn infer_extensions(&mut self) -> Result { - Ok(HashMap::new()) + pub fn infer_extensions(&mut self) -> Result<(), InferExtensionError> { + #[cfg(feature = "extension_inference")] + { + let solution = infer_extensions(self)?; + self.instantiate_extensions(solution); + } + Ok(()) } #[allow(dead_code)] diff --git a/quantinuum-hugr/src/hugr/validate/test.rs b/quantinuum-hugr/src/hugr/validate/test.rs index 80b0a7d63..267655c41 100644 --- a/quantinuum-hugr/src/hugr/validate/test.rs +++ b/quantinuum-hugr/src/hugr/validate/test.rs @@ -149,14 +149,15 @@ fn children_restrictions() { b.update_validate(&EMPTY_REG), Err(ValidationError::NonContainerWithChildren { node, .. }) => assert_eq!(node, copy) ); - let closure = b.infer_extensions().unwrap(); + + // After moving the previous definition to a valid place... b.set_parent(new_def, root); + b.infer_extensions().unwrap(); // Do this only while the Hugr is valid! - // After moving the previous definition to a valid place, - // add an input node to the module subgraph + // ...add an input node to the module subgraph let new_input = b.add_node_with_parent(root, ops::Input::new(type_row![])); assert_matches!( - b.validate_with_extension_closure(closure, &EMPTY_REG), + b.validate(&EMPTY_REG), Err(ValidationError::InvalidParentOp { parent, child, .. }) => {assert_eq!(parent, root); assert_eq!(child, new_input)} ); } @@ -591,8 +592,7 @@ mod extension_tests { .unwrap(); // Write Extension annotations into the Hugr while it's still well-formed // enough for us to compute them - let closure = b.infer_extensions().unwrap(); - b.instantiate_extensions(closure); + b.infer_extensions().unwrap(); b.validate(&EMPTY_REG).unwrap(); b.replace_op( copy, From 1d575e509d3e54cc5e8e15ecfb81bdde06c98f74 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 14:24:48 +0000 Subject: [PATCH 02/18] Do first main_loop (it mutates ctx, and may throw) --- quantinuum-hugr/src/extension/infer.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/quantinuum-hugr/src/extension/infer.rs b/quantinuum-hugr/src/extension/infer.rs index 29d9db74a..8944047d8 100644 --- a/quantinuum-hugr/src/extension/infer.rs +++ b/quantinuum-hugr/src/extension/infer.rs @@ -39,6 +39,7 @@ pub type ExtensionSolution = HashMap; /// concretise the extension requirements of the whole hugr. pub fn infer_extensions(hugr: &impl HugrView) -> Result { let mut ctx = UnificationContext::new(hugr); + ctx.main_loop()?; ctx.instantiate_variables(); ctx.main_loop() } From 5890873fa38928c4769e4ed5613ccf69e8fea0f8 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 14:35:07 +0000 Subject: [PATCH 03/18] clippy --- quantinuum-hugr/src/hugr.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/quantinuum-hugr/src/hugr.rs b/quantinuum-hugr/src/hugr.rs index 1d5bba5ec..4264c752c 100644 --- a/quantinuum-hugr/src/hugr.rs +++ b/quantinuum-hugr/src/hugr.rs @@ -8,8 +8,7 @@ pub mod serialize; pub mod validate; pub mod views; -#[cfg(not(feature = "extension_inference"))] -use std::collections::HashMap; + use std::collections::VecDeque; use std::iter; From b359731d573736a3a718c5c4d4ee1e3d353ebc54 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 14:40:47 +0000 Subject: [PATCH 04/18] fmt after clippy! --- quantinuum-hugr/src/hugr.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/quantinuum-hugr/src/hugr.rs b/quantinuum-hugr/src/hugr.rs index 4264c752c..32fdff34a 100644 --- a/quantinuum-hugr/src/hugr.rs +++ b/quantinuum-hugr/src/hugr.rs @@ -8,7 +8,6 @@ pub mod serialize; pub mod validate; pub mod views; - use std::collections::VecDeque; use std::iter; From 4fcb72f210d6d948fbb42f4ca6e6045a6703b96c Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 15:04:00 +0000 Subject: [PATCH 05/18] Update doc comment, don't mention closure --- quantinuum-hugr/src/extension/infer.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer.rs b/quantinuum-hugr/src/extension/infer.rs index 8944047d8..a26955e2b 100644 --- a/quantinuum-hugr/src/extension/infer.rs +++ b/quantinuum-hugr/src/extension/infer.rs @@ -31,12 +31,10 @@ use thiserror::Error; /// been inferred for their inputs. pub type ExtensionSolution = HashMap; -/// Infer extensions for a hugr. This is the main API exposed by this module +/// Infer extensions for a hugr. This is the main API exposed by this module. /// -/// Return a tuple of the solutions found for locations on the graph, and a -/// closure: a solution which would be valid if all of the variables in the graph -/// were instantiated to an empty extension set. This is used (by validation) to -/// concretise the extension requirements of the whole hugr. +/// Return all the solutions found for locations on the graph, these can be +/// written in by [`Hugr::instantiate_extensions`] pub fn infer_extensions(hugr: &impl HugrView) -> Result { let mut ctx = UnificationContext::new(hugr); ctx.main_loop()?; From 2390102c53d62408189b60c135aeeb6c580ffd5a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 15:04:17 +0000 Subject: [PATCH 06/18] instantiate_extensions: take &ExtensionSolution --- quantinuum-hugr/src/hugr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quantinuum-hugr/src/hugr.rs b/quantinuum-hugr/src/hugr.rs index 32fdff34a..9a60cf90d 100644 --- a/quantinuum-hugr/src/hugr.rs +++ b/quantinuum-hugr/src/hugr.rs @@ -209,14 +209,14 @@ impl Hugr { #[cfg(feature = "extension_inference")] { let solution = infer_extensions(self)?; - self.instantiate_extensions(solution); + self.instantiate_extensions(&solution); } Ok(()) } #[allow(dead_code)] /// Add extension requirement information to the hugr in place. - fn instantiate_extensions(&mut self, solution: ExtensionSolution) { + fn instantiate_extensions(&mut self, solution: &ExtensionSolution) { // We only care about inferred _input_ extensions, because `NodeType` // uses those to infer the output extensions for (node, input_extensions) in solution.iter() { From 5d0ffd360663f95340278bf2ec72460002e27014 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 15:04:35 +0000 Subject: [PATCH 07/18] Revert unnecessary test changes --- quantinuum-hugr/src/hugr/validate/test.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/quantinuum-hugr/src/hugr/validate/test.rs b/quantinuum-hugr/src/hugr/validate/test.rs index 267655c41..753766c9a 100644 --- a/quantinuum-hugr/src/hugr/validate/test.rs +++ b/quantinuum-hugr/src/hugr/validate/test.rs @@ -149,12 +149,11 @@ fn children_restrictions() { b.update_validate(&EMPTY_REG), Err(ValidationError::NonContainerWithChildren { node, .. }) => assert_eq!(node, copy) ); - - // After moving the previous definition to a valid place... + b.infer_extensions().unwrap(); b.set_parent(new_def, root); - b.infer_extensions().unwrap(); // Do this only while the Hugr is valid! - // ...add an input node to the module subgraph + // After moving the previous definition to a valid place, + // add an input node to the module subgraph let new_input = b.add_node_with_parent(root, ops::Input::new(type_row![])); assert_matches!( b.validate(&EMPTY_REG), From 777e8e1002e93a14afcff5dd31aa1520a8218321 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 15:17:49 +0000 Subject: [PATCH 08/18] Fix doclink --- quantinuum-hugr/src/extension/infer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quantinuum-hugr/src/extension/infer.rs b/quantinuum-hugr/src/extension/infer.rs index a26955e2b..f36f79a9f 100644 --- a/quantinuum-hugr/src/extension/infer.rs +++ b/quantinuum-hugr/src/extension/infer.rs @@ -34,7 +34,9 @@ pub type ExtensionSolution = HashMap; /// Infer extensions for a hugr. This is the main API exposed by this module. /// /// Return all the solutions found for locations on the graph, these can be -/// written in by [`Hugr::instantiate_extensions`] +/// passed to [`validate_with_extension_closure`] +/// +/// [`validate_with_extension_closure`]: crate::Hugr::validate_with_extension_closure pub fn infer_extensions(hugr: &impl HugrView) -> Result { let mut ctx = UnificationContext::new(hugr); ctx.main_loop()?; From 42a7d0509deca0b7dd909150ad081195c8e20bc0 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 18 Mar 2024 21:22:06 +0000 Subject: [PATCH 09/18] Add separate test of validate_with_extension_closure --- quantinuum-hugr/src/extension/infer/test.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 87b018f50..2d1c6afc2 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -794,6 +794,26 @@ fn test_cfg_loops() -> Result<(), Box> { Ok(()) } +#[test] +#[cfg(feature = "extension_inference")] +fn test_validate_with_closure() -> Result<(), Box> { + let hugr = make_looping_cfg( + ExtensionSet::new(), + ExtensionSet::singleton(&A), + ExtensionSet::singleton(&B), + )?; + let soln = infer_extensions(&hugr)?; + hugr.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; + let mut hugr = hugr.clone(); + assert_matches!( + hugr.validate(&PRELUDE_REGISTRY), + Err(ValidationError::ExtensionError(_)) + ); + hugr.update_validate(&PRELUDE_REGISTRY)?; // Solution written in, hence: + hugr.validate(&PRELUDE_REGISTRY)?; + Ok(()) +} + #[test] /// A control flow graph consisting of an entry node and a single block /// which adds a resource and links to both itself and the exit node. From 521314e59e6b3e6a90190d8405b9fcbb629f580e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 19 Mar 2024 09:02:47 +0000 Subject: [PATCH 10/18] Any clearer? --- quantinuum-hugr/src/extension/infer/test.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 2d1c6afc2..25f28bda7 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -797,18 +797,20 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - let hugr = make_looping_cfg( + let mut hugr = make_looping_cfg( ExtensionSet::new(), ExtensionSet::singleton(&A), ExtensionSet::singleton(&B), )?; - let soln = infer_extensions(&hugr)?; - hugr.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; - let mut hugr = hugr.clone(); assert_matches!( hugr.validate(&PRELUDE_REGISTRY), Err(ValidationError::ExtensionError(_)) ); + + let immut = hugr.clone(); + let soln = infer_extensions(&immut)?; + immut.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; + hugr.update_validate(&PRELUDE_REGISTRY)?; // Solution written in, hence: hugr.validate(&PRELUDE_REGISTRY)?; Ok(()) From 0ab3d34002a5e89b71f3f3eecf4555a40696798d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 27 Mar 2024 17:14:22 +0000 Subject: [PATCH 11/18] Do more of the test using 'immut' --- quantinuum-hugr/src/extension/infer/test.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 25f28bda7..c0cffea46 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -802,12 +802,13 @@ fn test_validate_with_closure() -> Result<(), Box> { ExtensionSet::singleton(&A), ExtensionSet::singleton(&B), )?; + let immut = hugr.clone(); + assert_matches!( - hugr.validate(&PRELUDE_REGISTRY), + immut.validate(&PRELUDE_REGISTRY), Err(ValidationError::ExtensionError(_)) ); - let immut = hugr.clone(); let soln = infer_extensions(&immut)?; immut.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; From a575aebe43dc315f279997e90a9a238eb329c83a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 8 Apr 2024 12:20:20 +0100 Subject: [PATCH 12/18] WIP extended test. Fails as I/O nodes not added --- quantinuum-hugr/src/extension/infer/test.rs | 78 +++++++++++++++++---- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index c0cffea46..8f57e8be9 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -797,23 +797,73 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - let mut hugr = make_looping_cfg( - ExtensionSet::new(), - ExtensionSet::singleton(&A), - ExtensionSet::singleton(&B), - )?; - let immut = hugr.clone(); - + use crate::utils::test_quantum_extension::{EXTENSION,EXTENSION_ID, h_gate}; + use crate::extension::prelude::{PRELUDE, PRELUDE_ID, PRELUDE_REGISTRY}; + use crate::hugr::hugrmut::sealed::HugrMutInternals; + use crate::extension::ExtensionRegistry; + let sig = FunctionType::new_endo(type_row![QB_T]).with_extension_delta(ExtensionSet::singleton(&EXTENSION_ID)); + let inner_open = { + let mut h = Hugr::new( + ops::DFG { signature: sig.clone() }.into(), + ); + let gate = h.add_node(h_gate().into()); + let [input, output] = h.get_io(h.root()).unwrap(); + h.connect(input,0,gate,0); + h.connect(gate,0,output,0); + h + }; assert_matches!( - immut.validate(&PRELUDE_REGISTRY), + inner_open.validate(&PRELUDE_REGISTRY), Err(ValidationError::ExtensionError(_)) ); - - let soln = infer_extensions(&immut)?; - immut.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; - - hugr.update_validate(&PRELUDE_REGISTRY)?; // Solution written in, hence: - hugr.validate(&PRELUDE_REGISTRY)?; + let soln = infer_extensions(&inner_open)?; + inner_open.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; + + // Building a Hugr around that inner DFG works, because inference on + // the outer Hugr includes inferring solutions for the inner. + let build_outer = |inner: Hugr| -> Hugr { + let mut outer = Hugr::new(ops::DFG { signature: sig.clone() }.into()); + let dfg_node = outer.insert_hugr(outer.root(), inner).new_root; + let [input, output] = outer.get_io(outer.root()).unwrap(); + outer.connect(input, 0, dfg_node, 0); + outer.connect(dfg_node, 0, output, 0); + outer + }; + let reg = ExtensionRegistry::try_new([PRELUDE.to_owned(), EXTENSION.to_owned()])?; + // Inserting uninferred version of 'inner' is OK + build_outer(inner_open.clone()).update_validate(®)?; + + // However, if we fix the solutions for the inner DFG to a 'wrong' value, + // then we cannot infer a solution for an outer DFG containing that: + let root = inner_open.root(); + + let inner_just_prelude = { + let mut h = inner_open.clone(); + h.replace_op(root, + NodeType::new( + h.get_optype(root).clone(), + ExtensionSet::singleton(&PRELUDE_ID) // maybe QB as well?? + )).unwrap(); + h.update_validate(®)?; // Writes 'wrong' solution into inner + h + }; + assert_matches!(build_outer(inner_just_prelude).update_validate(®), + Err(ValidationError::CantInfer(_))); + + // This does work if we fix the solutions to the inner DFG to the 'correct' + // value - but this requires knowing the input extensions before we start: + let inner_good = { + let mut h = inner_open.clone(); + h.replace_op(root, + NodeType::new( + h.get_optype(root).clone(), + ExtensionSet::from_iter([PRELUDE_ID, EXTENSION_ID]) // maybe QB as well?? + )).unwrap(); + h.update_validate(®)?; + h + }; + build_outer(inner_good).update_validate(®)?; + Ok(()) } From 58b312b6c06f94f355de604f4c8c666fbbd0cf81 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 8 Apr 2024 12:39:09 +0100 Subject: [PATCH 13/18] Fix test using builder --- quantinuum-hugr/src/extension/infer/test.rs | 89 ++++++++++++--------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 8f57e8be9..1e43c834d 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -797,73 +797,84 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - use crate::utils::test_quantum_extension::{EXTENSION,EXTENSION_ID, h_gate}; + use crate::builder::BuildError; + use crate::builder::FunctionBuilder; use crate::extension::prelude::{PRELUDE, PRELUDE_ID, PRELUDE_REGISTRY}; - use crate::hugr::hugrmut::sealed::HugrMutInternals; use crate::extension::ExtensionRegistry; - let sig = FunctionType::new_endo(type_row![QB_T]).with_extension_delta(ExtensionSet::singleton(&EXTENSION_ID)); + use crate::hugr::hugrmut::sealed::HugrMutInternals; + use crate::std_extensions::arithmetic::float_types; + use crate::utils::test_quantum_extension::EXTENSION_ID; + let sig = FunctionType::new_endo(type_row![QB_T]) + .with_extension_delta(ExtensionSet::singleton(&EXTENSION_ID)); let inner_open = { - let mut h = Hugr::new( - ops::DFG { signature: sig.clone() }.into(), - ); - let gate = h.add_node(h_gate().into()); - let [input, output] = h.get_io(h.root()).unwrap(); - h.connect(input,0,gate,0); - h.connect(gate,0,output,0); - h + let mut h = DFGBuilder::new(sig.clone())?; + let gate = h.add_dataflow_op( + LeafOp::Lift { + type_row: type_row![QB_T], + new_extension: EXTENSION_ID, + }, + h.input_wires(), + )?; + h.set_outputs(gate.outputs())?; + // Do not run inference yet + h.hugr().clone() }; + assert_matches!( inner_open.validate(&PRELUDE_REGISTRY), Err(ValidationError::ExtensionError(_)) ); + let soln = infer_extensions(&inner_open)?; inner_open.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; // Building a Hugr around that inner DFG works, because inference on // the outer Hugr includes inferring solutions for the inner. - let build_outer = |inner: Hugr| -> Hugr { - let mut outer = Hugr::new(ops::DFG { signature: sig.clone() }.into()); - let dfg_node = outer.insert_hugr(outer.root(), inner).new_root; - let [input, output] = outer.get_io(outer.root()).unwrap(); - outer.connect(input, 0, dfg_node, 0); - outer.connect(dfg_node, 0, output, 0); - outer + let build_outer = |inner: Hugr| -> Result { + // Using FunctionBuilder here because that forces the Input node + // to have empty input-extensions (unlike DFGBuilder) + let mut h = FunctionBuilder::new("main", sig.clone().into())?; + let dfg_node = h.add_hugr_with_wires(inner, h.input_wires())?; + h.set_outputs(dfg_node.outputs())?; + Ok(h.hugr().clone()) }; - let reg = ExtensionRegistry::try_new([PRELUDE.to_owned(), EXTENSION.to_owned()])?; - // Inserting uninferred version of 'inner' is OK - build_outer(inner_open.clone()).update_validate(®)?; - - // However, if we fix the solutions for the inner DFG to a 'wrong' value, + let reg = ExtensionRegistry::try_new([PRELUDE.to_owned(), float_types::EXTENSION.to_owned()])?; + + build_outer(inner_open.clone())?.update_validate(®)?; + + // However, if we fix the solutions for the inner DFG to a 'wrong' value + // (different from the actual annotation on the wire incoming to the inner DFG), // then we cannot infer a solution for an outer DFG containing that: let root = inner_open.root(); - - let inner_just_prelude = { + let inner_bad = { let mut h = inner_open.clone(); - h.replace_op(root, + h.replace_op( + root, NodeType::new( h.get_optype(root).clone(), - ExtensionSet::singleton(&PRELUDE_ID) // maybe QB as well?? - )).unwrap(); - h.update_validate(®)?; // Writes 'wrong' solution into inner + ExtensionSet::from_iter([PRELUDE_ID, EXTENSION_ID]), + ), + ) + .unwrap(); + h.update_validate(®).unwrap(); // Writes 'wrong' solution into inner h }; - assert_matches!(build_outer(inner_just_prelude).update_validate(®), - Err(ValidationError::CantInfer(_))); + assert_matches!( + build_outer(inner_bad)?.update_validate(®), + Err(ValidationError::CantInfer(_)) + ); // This does work if we fix the solutions to the inner DFG to the 'correct' - // value - but this requires knowing the input extensions before we start: + // value - of course this requires knowing the input extensions before we start: let inner_good = { let mut h = inner_open.clone(); - h.replace_op(root, - NodeType::new( - h.get_optype(root).clone(), - ExtensionSet::from_iter([PRELUDE_ID, EXTENSION_ID]) // maybe QB as well?? - )).unwrap(); + h.replace_op(root, NodeType::new_pure(h.get_optype(root).clone())) + .unwrap(); h.update_validate(®)?; h }; - build_outer(inner_good).update_validate(®)?; - + build_outer(inner_good)?.update_validate(®)?; + Ok(()) } From 56a4ad1ce845cd7bdd8aa4aac1f859f0a2717b68 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 9 Apr 2024 11:00:22 +0100 Subject: [PATCH 14/18] Move imports out --- quantinuum-hugr/src/extension/infer/test.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 1e43c834d..312f5086f 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -14,9 +14,15 @@ use crate::ops::{self, dataflow::IOTrait}; use crate::ops::{LeafOp, OpType}; #[cfg(feature = "extension_inference")] use crate::{ - builder::test::closed_dfg_root_hugr, - hugr::validate::ValidationError, + builder::{test::closed_dfg_root_hugr, BuildError, FunctionBuilder}, + extension::{ + prelude::{PRELUDE, PRELUDE_ID, PRELUDE_REGISTRY}, + ExtensionRegistry, + }, + hugr::{hugrmut::sealed::HugrMutInternals, validate::ValidationError}, ops::{dataflow::DataflowParent, handle::NodeHandle}, + std_extensions::arithmetic::float_types, + utils::test_quantum_extension::EXTENSION_ID, }; use crate::type_row; @@ -797,13 +803,6 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - use crate::builder::BuildError; - use crate::builder::FunctionBuilder; - use crate::extension::prelude::{PRELUDE, PRELUDE_ID, PRELUDE_REGISTRY}; - use crate::extension::ExtensionRegistry; - use crate::hugr::hugrmut::sealed::HugrMutInternals; - use crate::std_extensions::arithmetic::float_types; - use crate::utils::test_quantum_extension::EXTENSION_ID; let sig = FunctionType::new_endo(type_row![QB_T]) .with_extension_delta(ExtensionSet::singleton(&EXTENSION_ID)); let inner_open = { From 2becf67cc181a94c2fac3198727c88dce7acf126 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 9 Apr 2024 11:38:53 +0100 Subject: [PATCH 15/18] No need to lift, drop EXTENSION_ID, tidies --- quantinuum-hugr/src/extension/infer/test.rs | 25 ++++++--------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 312f5086f..a8305a2c6 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -16,13 +16,12 @@ use crate::ops::{LeafOp, OpType}; use crate::{ builder::{test::closed_dfg_root_hugr, BuildError, FunctionBuilder}, extension::{ - prelude::{PRELUDE, PRELUDE_ID, PRELUDE_REGISTRY}, + prelude::{PRELUDE, PRELUDE_ID}, ExtensionRegistry, }, hugr::{hugrmut::sealed::HugrMutInternals, validate::ValidationError}, ops::{dataflow::DataflowParent, handle::NodeHandle}, std_extensions::arithmetic::float_types, - utils::test_quantum_extension::EXTENSION_ID, }; use crate::type_row; @@ -803,18 +802,10 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - let sig = FunctionType::new_endo(type_row![QB_T]) - .with_extension_delta(ExtensionSet::singleton(&EXTENSION_ID)); + let sig = FunctionType::new_endo(type_row![QB_T]); let inner_open = { let mut h = DFGBuilder::new(sig.clone())?; - let gate = h.add_dataflow_op( - LeafOp::Lift { - type_row: type_row![QB_T], - new_extension: EXTENSION_ID, - }, - h.input_wires(), - )?; - h.set_outputs(gate.outputs())?; + h.set_outputs(h.input_wires())?; // Do not run inference yet h.hugr().clone() }; @@ -851,11 +842,10 @@ fn test_validate_with_closure() -> Result<(), Box> { root, NodeType::new( h.get_optype(root).clone(), - ExtensionSet::from_iter([PRELUDE_ID, EXTENSION_ID]), + ExtensionSet::from_iter([PRELUDE_ID]), ), - ) - .unwrap(); - h.update_validate(®).unwrap(); // Writes 'wrong' solution into inner + )?; + h.update_validate(®)?; // Writes 'wrong' solution into inner h }; assert_matches!( @@ -867,8 +857,7 @@ fn test_validate_with_closure() -> Result<(), Box> { // value - of course this requires knowing the input extensions before we start: let inner_good = { let mut h = inner_open.clone(); - h.replace_op(root, NodeType::new_pure(h.get_optype(root).clone())) - .unwrap(); + h.replace_op(root, NodeType::new_pure(h.get_optype(root).clone()))?; h.update_validate(®)?; h }; From 21db3a6c105c4a35c9d97aac77cffdb89efb000f Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 9 Apr 2024 11:53:47 +0100 Subject: [PATCH 16/18] rewrite test w/closed_dfg_root hugr; fix: infer_extensions returns only NEW results --- quantinuum-hugr/src/extension/infer.rs | 7 +- quantinuum-hugr/src/extension/infer/test.rs | 125 ++++++++++++-------- 2 files changed, 82 insertions(+), 50 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer.rs b/quantinuum-hugr/src/extension/infer.rs index f36f79a9f..46387ab4f 100644 --- a/quantinuum-hugr/src/extension/infer.rs +++ b/quantinuum-hugr/src/extension/infer.rs @@ -41,7 +41,12 @@ pub fn infer_extensions(hugr: &impl HugrView) -> Result Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { + const EXT_ID: ExtensionId = ExtensionId::new_unchecked("foo"); let sig = FunctionType::new_endo(type_row![QB_T]); let inner_open = { - let mut h = DFGBuilder::new(sig.clone())?; - h.set_outputs(h.input_wires())?; - // Do not run inference yet - h.hugr().clone() + let mut h = closed_dfg_root_hugr(sig.clone()); + h.replace_op(h.root(), NodeType::new_open(h.get_optype(h.root()).clone()))?; + let [input, output] = h.get_io(h.root()).unwrap(); + h.connect(input, 0, output, 0); + h }; - assert_matches!( - inner_open.validate(&PRELUDE_REGISTRY), - Err(ValidationError::ExtensionError(_)) - ); - - let soln = infer_extensions(&inner_open)?; - inner_open.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; - - // Building a Hugr around that inner DFG works, because inference on - // the outer Hugr includes inferring solutions for the inner. - let build_outer = |inner: Hugr| -> Result { - // Using FunctionBuilder here because that forces the Input node - // to have empty input-extensions (unlike DFGBuilder) - let mut h = FunctionBuilder::new("main", sig.clone().into())?; - let dfg_node = h.add_hugr_with_wires(inner, h.input_wires())?; - h.set_outputs(dfg_node.outputs())?; - Ok(h.hugr().clone()) + let inner_prelude = { + let mut h = inner_open.clone(); + h.replace_op( + h.root(), + NodeType::new( + h.get_optype(h.root()).clone(), + ExtensionSet::singleton(&PRELUDE_ID), + ), + )?; + h }; - let reg = ExtensionRegistry::try_new([PRELUDE.to_owned(), float_types::EXTENSION.to_owned()])?; - build_outer(inner_open.clone())?.update_validate(®)?; - - // However, if we fix the solutions for the inner DFG to a 'wrong' value - // (different from the actual annotation on the wire incoming to the inner DFG), - // then we cannot infer a solution for an outer DFG containing that: - let root = inner_open.root(); - let inner_bad = { + let inner_other = { let mut h = inner_open.clone(); h.replace_op( - root, + h.root(), NodeType::new( - h.get_optype(root).clone(), - ExtensionSet::from_iter([PRELUDE_ID]), + h.get_optype(h.root()).clone(), + ExtensionSet::singleton(&EXT_ID), ), )?; - h.update_validate(®)?; // Writes 'wrong' solution into inner h }; + + // All three can be inferred and validated, without writing solutions in: + for inner in [&inner_open, &inner_prelude, &inner_other] { + assert_matches!( + inner.validate(&PRELUDE_REGISTRY), + Err(ValidationError::ExtensionError(_)) + ); + + let soln = infer_extensions(inner)?; + inner.validate_with_extension_closure(soln, &PRELUDE_REGISTRY)?; + } + + // Helper builds a Hugr with extensions {PRELUDE_ID}, around argument + let build_outer_prelude = |inner: Hugr| -> Hugr { + let mut h = closed_dfg_root_hugr(sig.clone()); + h.replace_op( + h.root(), + NodeType::new( + h.get_optype(h.root()).clone(), + ExtensionSet::singleton(&PRELUDE_ID), + ), + ) + .unwrap(); + let [input, output] = h.get_io(h.root()).unwrap(); + let inner_node = h.insert_hugr(h.root(), inner).new_root; + h.connect(input, 0, inner_node, 0); + h.connect(inner_node, 0, output, 0); + h + }; + + // Building a Hugr around the inner DFG works if the inner DFG is open, + // or has the correct (prelude) extensions: + for inner in [&inner_open, &inner_prelude] { + let mut h = build_outer_prelude(inner.clone()); + h.update_validate(&PRELUDE_REGISTRY)?; + } + + // ...but fails if the inner DFG already has the 'wrong' extensions: + //let reg = ExtensionRegistry::try_new([Extension::new(EXT_ID), PRELUDE.to_owned()])?; assert_matches!( - build_outer(inner_bad)?.update_validate(®), + build_outer_prelude(inner_other.clone()).update_validate(&PRELUDE_REGISTRY), Err(ValidationError::CantInfer(_)) ); - // This does work if we fix the solutions to the inner DFG to the 'correct' - // value - of course this requires knowing the input extensions before we start: - let inner_good = { - let mut h = inner_open.clone(); - h.replace_op(root, NodeType::new_pure(h.get_optype(root).clone()))?; - h.update_validate(®)?; - h - }; - build_outer(inner_good)?.update_validate(®)?; + // If we do inference on the inner Hugr first, this works if the + // inner DFG already had the correct input-extensions: + let mut inner_prelude = inner_prelude.clone(); + inner_prelude.update_validate(&PRELUDE_REGISTRY)?; + build_outer_prelude(inner_prelude).update_validate(&PRELUDE_REGISTRY)?; + + // But fails even for previously-open inner DFG as inference + // infers an incorrect (empty) solution: + let mut inner_inferred = inner_open; + inner_inferred.update_validate(&PRELUDE_REGISTRY)?; + assert_matches!( + build_outer_prelude(inner_inferred).update_validate(&PRELUDE_REGISTRY), + Err(ValidationError::CantInfer(_)) + ); Ok(()) } From e39e325e5314f6c4bf3431dd620c2539474fe728 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 9 Apr 2024 12:01:47 +0100 Subject: [PATCH 17/18] common up via helpers --- quantinuum-hugr/src/extension/infer/test.rs | 55 ++++++--------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index 21a4c2658..e4cb5134d 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -798,39 +798,26 @@ fn test_cfg_loops() -> Result<(), Box> { #[test] #[cfg(feature = "extension_inference")] fn test_validate_with_closure() -> Result<(), Box> { - const EXT_ID: ExtensionId = ExtensionId::new_unchecked("foo"); - let sig = FunctionType::new_endo(type_row![QB_T]); - let inner_open = { - let mut h = closed_dfg_root_hugr(sig.clone()); - h.replace_op(h.root(), NodeType::new_open(h.get_optype(h.root()).clone()))?; + fn dfg_hugr_with_exts(e: Option) -> (Hugr, Node, Node) { + let mut h = closed_dfg_root_hugr(FunctionType::new_endo(type_row![QB_T])); + h.replace_op(h.root(), NodeType::new(h.get_optype(h.root()).clone(), e)) + .unwrap(); let [input, output] = h.get_io(h.root()).unwrap(); + (h, input, output) + } + fn identity_hugr_with_exts(e: Option) -> Hugr { + let (mut h, input, output) = dfg_hugr_with_exts(e); h.connect(input, 0, output, 0); h - }; + } - let inner_prelude = { - let mut h = inner_open.clone(); - h.replace_op( - h.root(), - NodeType::new( - h.get_optype(h.root()).clone(), - ExtensionSet::singleton(&PRELUDE_ID), - ), - )?; - h - }; + const EXT_ID: ExtensionId = ExtensionId::new_unchecked("foo"); - let inner_other = { - let mut h = inner_open.clone(); - h.replace_op( - h.root(), - NodeType::new( - h.get_optype(h.root()).clone(), - ExtensionSet::singleton(&EXT_ID), - ), - )?; - h - }; + let inner_open = identity_hugr_with_exts(None); + + let inner_prelude = identity_hugr_with_exts(Some(ExtensionSet::singleton(&PRELUDE_ID))); + + let inner_other = identity_hugr_with_exts(Some(ExtensionSet::singleton(&EXT_ID))); // All three can be inferred and validated, without writing solutions in: for inner in [&inner_open, &inner_prelude, &inner_other] { @@ -845,16 +832,7 @@ fn test_validate_with_closure() -> Result<(), Box> { // Helper builds a Hugr with extensions {PRELUDE_ID}, around argument let build_outer_prelude = |inner: Hugr| -> Hugr { - let mut h = closed_dfg_root_hugr(sig.clone()); - h.replace_op( - h.root(), - NodeType::new( - h.get_optype(h.root()).clone(), - ExtensionSet::singleton(&PRELUDE_ID), - ), - ) - .unwrap(); - let [input, output] = h.get_io(h.root()).unwrap(); + let (mut h, input, output) = dfg_hugr_with_exts(Some(ExtensionSet::singleton(&PRELUDE_ID))); let inner_node = h.insert_hugr(h.root(), inner).new_root; h.connect(input, 0, inner_node, 0); h.connect(inner_node, 0, output, 0); @@ -869,7 +847,6 @@ fn test_validate_with_closure() -> Result<(), Box> { } // ...but fails if the inner DFG already has the 'wrong' extensions: - //let reg = ExtensionRegistry::try_new([Extension::new(EXT_ID), PRELUDE.to_owned()])?; assert_matches!( build_outer_prelude(inner_other.clone()).update_validate(&PRELUDE_REGISTRY), Err(ValidationError::CantInfer(_)) From 7607a9a21a896c7b612c3cda15f57d5d90de3965 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 9 Apr 2024 12:07:24 +0100 Subject: [PATCH 18/18] Rename, avoid a clone --- quantinuum-hugr/src/extension/infer/test.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/quantinuum-hugr/src/extension/infer/test.rs b/quantinuum-hugr/src/extension/infer/test.rs index e4cb5134d..256480614 100644 --- a/quantinuum-hugr/src/extension/infer/test.rs +++ b/quantinuum-hugr/src/extension/infer/test.rs @@ -852,13 +852,13 @@ fn test_validate_with_closure() -> Result<(), Box> { Err(ValidationError::CantInfer(_)) ); - // If we do inference on the inner Hugr first, this works if the + // If we do inference on the inner Hugr first, this (still) works if the // inner DFG already had the correct input-extensions: - let mut inner_prelude = inner_prelude.clone(); - inner_prelude.update_validate(&PRELUDE_REGISTRY)?; - build_outer_prelude(inner_prelude).update_validate(&PRELUDE_REGISTRY)?; + let mut inner_prelude_inferred = inner_prelude; + inner_prelude_inferred.update_validate(&PRELUDE_REGISTRY)?; + build_outer_prelude(inner_prelude_inferred).update_validate(&PRELUDE_REGISTRY)?; - // But fails even for previously-open inner DFG as inference + // But fails for previously-open inner DFG as inference // infers an incorrect (empty) solution: let mut inner_inferred = inner_open; inner_inferred.update_validate(&PRELUDE_REGISTRY)?;