From c564b806c9ff31d5faa6aa3f02c18cfe6a5ff2c5 Mon Sep 17 00:00:00 2001 From: Julien Gacon Date: Fri, 30 Aug 2024 15:57:47 +0200 Subject: [PATCH] Debug: disable cache trying to figure out why the windows CI fails (after being unable to locally reproduce we're using CI with a reduced set of tests) --- azure-pipelines.yml | 65 ----- crates/accelerate/src/commutation_checker.rs | 274 +++++++++---------- 2 files changed, 137 insertions(+), 202 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c3154abf412c..ac45bea36990 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -136,30 +136,6 @@ stages: # merge queue, where they'll use the next rule as the branch-protection rule # for the final merge to the base branch. - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: - # The preliminary stage should be small in both total runtime (including - # provisioning) and resources required. About half of PR commits result in - # a CI failure, and over 90% of these are in linting, documention or a test - # failure that would affect _any_ OS or Python version. The goal in the - # first stage is to catch the vast majority of failures with minimal cost. - - stage: "Lint_Docs_Prelim_Tests" - displayName: "Preliminary tests" - jobs: - - template: ".azure/lint_docs_qpy-linux.yml" - parameters: - pythonVersion: ${{ parameters.minimumPythonVersion }} - - - template: ".azure/test-linux.yml" - parameters: - pythonVersion: ${{ parameters.minimumPythonVersion }} - # A PR is more likely to fail CI because it introduces a logic error - # into an existing test than because it adds a new test / optional - # dependency that isn't accounted for in the test-skipping logic - # (and such a failure would need fewer iterations to fix). We want - # to fail fast in the first CI stage. - installOptionals: true - testRust: true - testImages: true - # The rest of the PR pipeline is to test the oldest and newest supported # versions of Python. It's very rare for a failure to be specific to an # intermediate version of Python, so we just catch those in the cron-job @@ -168,24 +144,6 @@ stages: displayName: "Main tests" dependsOn: "Lint_Docs_Prelim_Tests" jobs: - - template: ".azure/test-linux.yml" - parameters: - pythonVersion: ${{ parameters.maximumPythonVersion }} - testRust: false - testImages: false - installFromSdist: true - installOptionals: false - - - template: ".azure/test-macos.yml" - parameters: - pythonVersion: ${{ parameters.minimumPythonVersion }} - installOptionals: true - - - template: ".azure/test-macos.yml" - parameters: - pythonVersion: ${{ parameters.maximumPythonVersion }} - installOptionals: false - - template: ".azure/test-windows.yml" parameters: pythonVersion: ${{ parameters.minimumPythonVersion }} @@ -208,29 +166,6 @@ stages: - stage: "Merge_Queue" displayName: "Merge queue" jobs: - - template: ".azure/lint_docs_qpy-linux.yml" - parameters: - pythonVersion: ${{ parameters.minimumPythonVersion }} - - - template: ".azure/test-linux.yml" - parameters: - pythonVersion: ${{ parameters.minimumPythonVersion }} - installOptionals: true - testRust: true - testImages: true - - - template: ".azure/test-linux.yml" - parameters: - pythonVersion: ${{ parameters.maximumPythonVersion }} - installOptionals: false - testRust: false - testImages: false - - - template: ".azure/test-macos.yml" - parameters: - pythonVersion: ${{ parameters.maximumPythonVersion }} - installOptionals: false - - template: ".azure/test-windows.yml" parameters: pythonVersion: ${{ parameters.maximumPythonVersion }} diff --git a/crates/accelerate/src/commutation_checker.rs b/crates/accelerate/src/commutation_checker.rs index c23c447c632a..73c65c260103 100644 --- a/crates/accelerate/src/commutation_checker.rs +++ b/crates/accelerate/src/commutation_checker.rs @@ -35,7 +35,7 @@ use qiskit_circuit::operations::{Operation, OperationRef, Param}; use qiskit_circuit::{BitType, Clbit, Qubit}; static SKIPPED_NAMES: [&str; 4] = ["measure", "reset", "delay", "initialize"]; -static NO_CACHE_NAMES: [&str; 2] = ["annotated", "linear_function"]; +// static NO_CACHE_NAMES: [&str; 2] = ["annotated", "linear_function"]; static SUPPORTED_OP: Lazy> = Lazy::new(|| { HashSet::from([ "h", "x", "y", "z", "sx", "sxdg", "t", "tdg", "s", "sdg", "cx", "cy", "cz", "swap", @@ -258,55 +258,14 @@ impl CommutationChecker { (qargs1, qargs2) }; - let skip_cache: bool = NO_CACHE_NAMES.contains(&first_op.name()) || - NO_CACHE_NAMES.contains(&second_op.name()) || - // Skip params that do not evaluate to floats for caching and commutation library - first_params.iter().any(|p| !matches!(p, Param::Float(_))) || - second_params.iter().any(|p| !matches!(p, Param::Float(_))); - - if skip_cache { - return self.commute_matmul( - py, - first_op, - first_params, - first_qargs, - second_op, - second_params, - second_qargs, - ); - } - - // Query commutation library - if let Some(is_commuting) = - self.library - .check_commutation_entries(first_op, first_qargs, second_op, second_qargs) - { - return Ok(is_commuting); - } - // Query cache - match self - .cache - .get(&(first_op.name().to_string(), second_op.name().to_string())) - { - Some(commutation_dict) => { - let placement = get_relative_placement(first_qargs, second_qargs); - let hashes = ( - hashable_params(first_params), - hashable_params(second_params), - ); - match commutation_dict.get(&(placement, hashes)) { - Some(commutation) => { - self._cache_hit += 1; - return Ok(*commutation); - } - None => self._cache_miss += 1, - } - } - None => self._cache_miss += 1, - } + // let skip_cache: bool = NO_CACHE_NAMES.contains(&first_op.name()) || + // NO_CACHE_NAMES.contains(&second_op.name()) || + // // Skip params that do not evaluate to floats for caching and commutation library + // first_params.iter().any(|p| !matches!(p, Param::Float(_))) || + // second_params.iter().any(|p| !matches!(p, Param::Float(_))); - // Perform matrix multiplication to determine commutation - let is_commuting = self.commute_matmul( + // if skip_cache { + return self.commute_matmul( py, first_op, first_params, @@ -314,40 +273,81 @@ impl CommutationChecker { second_op, second_params, second_qargs, - )?; - - // TODO: implement a LRU cache for this - if self.current_cache_entries >= self.cache_max_entries { - self.clear_cache(); - } - // Cache results from is_commuting - self.cache - .entry((first_op.name().to_string(), second_op.name().to_string())) - .and_modify(|entries| { - let key = ( - get_relative_placement(first_qargs, second_qargs), - ( - hashable_params(first_params), - hashable_params(second_params), - ), - ); - entries.insert(key, is_commuting); - self.current_cache_entries += 1; - }) - .or_insert_with(|| { - let mut entries = HashMap::with_capacity(1); - let key = ( - get_relative_placement(first_qargs, second_qargs), - ( - hashable_params(first_params), - hashable_params(second_params), - ), - ); - entries.insert(key, is_commuting); - self.current_cache_entries += 1; - CommutationCacheEntry { mapping: entries } - }); - Ok(is_commuting) + ); + // } + + // // Query commutation library + // if let Some(is_commuting) = + // self.library + // .check_commutation_entries(first_op, first_qargs, second_op, second_qargs) + // { + // return Ok(is_commuting); + // } + // // Query cache + // match self + // .cache + // .get(&(first_op.name().to_string(), second_op.name().to_string())) + // { + // Some(commutation_dict) => { + // let placement = get_relative_placement(first_qargs, second_qargs); + // let hashes = ( + // hashable_params(first_params), + // hashable_params(second_params), + // ); + // match commutation_dict.get(&(placement, hashes)) { + // Some(commutation) => { + // self._cache_hit += 1; + // return Ok(*commutation); + // } + // None => self._cache_miss += 1, + // } + // } + // None => self._cache_miss += 1, + // } + + // // Perform matrix multiplication to determine commutation + // let is_commuting = self.commute_matmul( + // py, + // first_op, + // first_params, + // first_qargs, + // second_op, + // second_params, + // second_qargs, + // )?; + + // // TODO: implement a LRU cache for this + // if self.current_cache_entries >= self.cache_max_entries { + // self.clear_cache(); + // } + // // Cache results from is_commuting + // self.cache + // .entry((first_op.name().to_string(), second_op.name().to_string())) + // .and_modify(|entries| { + // let key = ( + // get_relative_placement(first_qargs, second_qargs), + // ( + // hashable_params(first_params), + // hashable_params(second_params), + // ), + // ); + // entries.insert(key, is_commuting); + // self.current_cache_entries += 1; + // }) + // .or_insert_with(|| { + // let mut entries = HashMap::with_capacity(1); + // let key = ( + // get_relative_placement(first_qargs, second_qargs), + // ( + // hashable_params(first_params), + // hashable_params(second_params), + // ), + // ); + // entries.insert(key, is_commuting); + // self.current_cache_entries += 1; + // CommutationCacheEntry { mapping: entries } + // }); + // Ok(is_commuting) } #[allow(clippy::too_many_arguments)] @@ -545,21 +545,21 @@ where .any(|x| matches!(x, Param::ParameterExpression(_))) } -fn get_relative_placement( - first_qargs: &[Qubit], - second_qargs: &[Qubit], -) -> SmallVec<[Option; 2]> { - let qubits_g2: HashMap<_, _> = second_qargs - .iter() - .enumerate() - .map(|(i_g1, q_g1)| (q_g1, Qubit(i_g1 as u32))) - .collect(); - - first_qargs - .iter() - .map(|q_g0| qubits_g2.get(q_g0).copied()) - .collect() -} +// fn get_relative_placement( +// first_qargs: &[Qubit], +// second_qargs: &[Qubit], +// ) -> SmallVec<[Option; 2]> { +// let qubits_g2: HashMap<_, _> = second_qargs +// .iter() +// .enumerate() +// .map(|(i_g1, q_g1)| (q_g1, Qubit(i_g1 as u32))) +// .collect(); + +// first_qargs +// .iter() +// .map(|q_g0| qubits_g2.get(q_g0).copied()) +// .collect() +// } #[derive(Clone, Debug)] #[pyclass] @@ -567,27 +567,27 @@ pub struct CommutationLibrary { pub library: Option>, } -impl CommutationLibrary { - fn check_commutation_entries( - &self, - first_op: &OperationRef, - first_qargs: &[Qubit], - second_op: &OperationRef, - second_qargs: &[Qubit], - ) -> Option { - if let Some(library) = &self.library { - match library.get(&(first_op.name().to_string(), second_op.name().to_string())) { - Some(CommutationLibraryEntry::Commutes(b)) => Some(*b), - Some(CommutationLibraryEntry::QubitMapping(qm)) => qm - .get(&get_relative_placement(first_qargs, second_qargs)) - .copied(), - _ => None, - } - } else { - None - } - } -} +// impl CommutationLibrary { +// fn check_commutation_entries( +// &self, +// first_op: &OperationRef, +// first_qargs: &[Qubit], +// second_op: &OperationRef, +// second_qargs: &[Qubit], +// ) -> Option { +// if let Some(library) = &self.library { +// match library.get(&(first_op.name().to_string(), second_op.name().to_string())) { +// Some(CommutationLibraryEntry::Commutes(b)) => Some(*b), +// Some(CommutationLibraryEntry::QubitMapping(qm)) => qm +// .get(&get_relative_placement(first_qargs, second_qargs)) +// .copied(), +// _ => None, +// } +// } else { +// None +// } +// } +// } #[pymethods] impl CommutationLibrary { @@ -661,16 +661,16 @@ struct CommutationCacheEntry { mapping: HashMap, } impl CommutationCacheEntry { - fn get(&self, key: &CacheKey) -> Option<&bool> { - self.mapping.get(key) - } + // fn get(&self, key: &CacheKey) -> Option<&bool> { + // self.mapping.get(key) + // } fn iter(&self) -> Iter<'_, CacheKey, bool> { self.mapping.iter() } - fn insert(&mut self, k: CacheKey, v: bool) -> Option { - self.mapping.insert(k, v) - } + // fn insert(&mut self, k: CacheKey, v: bool) -> Option { + // self.mapping.insert(k, v) + // } } impl ToPyObject for CommutationCacheEntry { @@ -737,18 +737,18 @@ impl PartialEq for ParameterKey { impl Eq for ParameterKey {} -fn hashable_params(params: &[Param]) -> SmallVec<[ParameterKey; 3]> { - params - .iter() - .map(|x| { - if let Param::Float(x) = x { - ParameterKey(*x) - } else { - panic!("Unable to hash a non-float instruction parameter.") - } - }) - .collect() -} +// fn hashable_params(params: &[Param]) -> SmallVec<[ParameterKey; 3]> { +// params +// .iter() +// .map(|x| { +// if let Param::Float(x) = x { +// ParameterKey(*x) +// } else { +// panic!("Unable to hash a non-float instruction parameter.") +// } +// }) +// .collect() +// } #[pymodule] pub fn commutation_checker(m: &Bound) -> PyResult<()> {