-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add more heuristic to SymbolExpr #14548
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
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Robin Adams <robinad@chalmers.se>
* Fixes to various graphs in the docs * Fix a bug caused by `tight_layout` crashing on some images * Apply the fix to the timeline plotter * Remove release note and fix bloch sphere visualization * Additional bloch fix * Slighly relax image compairson for bloch sphere due to small discrepancies between the server and local versions.
* Add random_clifford_circuit to API docs * Add it also to the circuit module doc
* Remove Python dependency for creating a blank dag This commit removes the py token argument for the dag constructor which wasn't strictly needed. The only pieces of the dagcircuit struct which need python is the metadata atribute (which is an `Option<PyDict>`) and the var tracking sets. This commit rearranges the constructor so it doesn't actively need python. This is achieved by moving the var tracking sets inside a once lock which is only initialized in a context where vars are being used. The metadata is changed to None by default. The other use case was the control flow module which wasn't strictly required and is removed in this PR and using the import once cell mechanism that we use for this everywhere else. * Remove conditional import of OnceLock since it's always used * Update rust tests * Remove _bound suffix from method names * Add docstring to VarsByType
Luciano had an old IBM email address that is no longer active. We were previously canonicalising to that email, rather than his current one.
* Update security policy for 1.x and 2.x With the 1.x release we're extending security support for Qiskit to 1 yr. We still only support 1.x for general bugfixes for 6 months after the 2.0.0 release. But for 1.x if any security vulnerabilities are identified we will provide fixes for that up to 1 yr after the release of 2.0.0. This was reflected in the 1.4.0 release notes and is on the version strategy docs: https://docs.quantum.ibm.com/open-source/qiskit-sdk-version-strategy but we forgot to update the security policy document in the Qiskit repo. This commit fixes this oversight. * Update SECURITY.md Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* add missing branch for global phase update in basis_translator * Adding test * reno
* add empty stubs * add mcx_linear_depth implementation, add docstrings to all 4 * add mcx_log_depth implementation * typo in function name * remove gate.definition uses, work with separate qregs * fix linter errors * fix type hints, whitespace * replace assert with valueerror * fix linter errors * another linter error * add new test file for mcx_synthesis; test statevector correctness * add test for cx count * simplify loop logic * add test for depth * add module docstring * add header * replace assert with valueerror * minor linter error * bugfix: typo in dirty anc. case * minor refactoring + docstrings * add releasenote * add Raises to dosctrings * Apply suggestions from code review minor suggestions Co-authored-by: Julien Gacon <gaconju@gmail.com> * add empty stubs * add mcx_linear_depth implementation, add docstrings to all 4 * add mcx_log_depth implementation * typo in function name * remove gate.definition uses, work with separate qregs * fix linter errors * fix type hints, whitespace * replace assert with valueerror * fix linter errors * another linter error * add new test file for mcx_synthesis; test statevector correctness * add test for cx count * simplify loop logic * add test for depth * add module docstring * add header * replace assert with valueerror * minor linter error * bugfix: typo in dirty anc. case * rebase main * add releasenote * add Raises to dosctrings * Apply suggestions from code review minor suggestions Co-authored-by: Julien Gacon <gaconju@gmail.com> * whitespace * throw error when n_ctrls <=2 and linear_ladder_ops now accepts int instead of list * move tests to new file * black formatting * small ctrl vals, large n_ctrls timeout on windows CI * Update releasenotes/notes/better_mcx_synthesis-7e6e265147bc1d33.yaml Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> * spelling * whitespace --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
…` is set (Qiskit#14065) * Fix dt oversight in generate_preset_pass_manager * Update warning to mention explicitly the invalidation of custom durations and error rates. * Do not overwrite original backend values inside transpile, iterate over target to reset internal instruction_durations value so that new dt can be taken into account * One iteration is enough to reset the value of self._instruction_durations * Estimate duration instead of hardcoding it * Apply suggestion from Matt's code review Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Fix example * Move cache invalidation to target dt setter. * Add reno --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Configure reno for C and remove legacy algorithm mentions * add deprecations_c, keep algo sections
* updates to synth_mcx_n_dirty_i15 and to synth_mcx_1_clean_b95 * add explicit tests for MCX synthesis algorithms * improving comment * rename variable * slightly generalize and simplify _msu_real_diagonal * updating qasm tests * improving treatment of special cases in synth_mcx_n_dirty_i15 * remove duplicated tests * add tests for counting CX gates * add some more cx_count tests * remove tests * improving docstring comments (minor) * pass over tests * docstring * increasing the number of control qubits in some tests * changing circuit's name to mcx_vchain for compatibility * cleanup * fixing MCX qasms * updating docstrings * reno * tests and docs * fix qasm tests * addressing review comments * futher expanding review comment --------- Co-authored-by: ShellyGarion <shelly@il.ibm.com>
* Consistent sparse list format in sparse operators * review comments - use combine in test - fix comment
Originally, I thought it'd make the most sense to reject construction of operations that'd have a duration type but also be non-const expressions. But, a user can currently create a `Var` with a duration type, so this seems like an incomplete restriction.
Normally creating a custom gate class that overloads the name of a Qiskit defined operation is not valid and not allowed. The names have meaning and are often used as identifiers and this overloading the name will prevent Qiskit from correctly identifying an operation. However as was discovered in Qiskit#14103 there are some paths available around serialization and I/O where Qiskit does this itself. For example, qasm (both 2 and 3) is a lossy serialization format and qasm2 doesn't have a representation of a UnitaryGate. So when the qasm2 exporter encounteres a `UnitaryGate` it is serialized as a custom gate definition with the name "unitary" in the output qasm2 and the definition is a decomposition of the unitary from the `UnitaryGate`. When that qasm2 program is subsequently deserialized by qiskit parser the custom gate named "unitary" is added as a `_DefinedGate` subclass which includes an `__array__` implementation which computes the unitary from the definition using the quantum info Operator class. This makes the custom gate parsed from qasm2 look like a `UnitaryGate` despite not actually one so this is typically fine for most use cases. However, since Qiskit#13759 trying to add that not `UnitaryGate` object named "unitary" would cause the Python -> Rust translation to panic (which happens as part of qasm2 desierailzation). because the conversion was expecting a gate named `unitary` to be a `UnitaryGate` as is prescribed by the data model. This commit fixes this by gracefully handling the lack of a matrix parameter as it not actually being a `UnitaryGate` and instead the object gets treated as a `PyGate` in rust which is the expected behavior. Related to Qiskit#14103
* Explain circlib design e.g. why we use different gate types and what's the advantages * separate old circuits and new gates * alt text & broken links * Apply suggestions from code review Thank you Elena! Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> * more review comments --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Add MCMXGate to custom operations * Add test and reno --------- Co-authored-by: Shelly Garion <46566946+ShellyGarion@users.noreply.github.com>
* Get C API docs building Co-authored-by: Max Rossmannek <rmax@ethz.ch> Co-authored-by: Julien Gacon <jul@zurich.ibm.com> * Separate `docs` from `docs-c` in `tox.ini` * Move each group to its own page * Reorganise docs page layout * Fix some group namings * Get enums working * Update tox docs-clean * Add exit codes page * Add "Data types" heading * Update docs/cdoc/index.rst Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * QkObs docs and nits * add bit term note that the Members section should be updated to match doxygen's enum style before merging, the table is a placeholder * Re-format index RST * update members * iterating on layout * better enums (follow Glib style because it looks great) * fix linebreaks * minor fixes (brackets, missing whitespace) * try fix tables * Fix RST formatting quirks * Install doxygen in CI * document exit codes * rm trailing ] * more enum work * Fix table * QkExitCode and remove rustdoc links * Rename exit codes page * Update docs/cdoc/index.rst Co-authored-by: Julien Gacon <gaconju@gmail.com> * Fix xrefs and toc * Fix CI? * Remove bad xrefs * Update docs/cdoc/qk-obs.rst * Document doxygen is required for docs builds now --------- Co-authored-by: Max Rossmannek <rmax@ethz.ch> Co-authored-by: Julien Gacon <jul@zurich.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Julien Gacon <jules.gacon@googlemail.com> Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Run C tests on all tier 1 platforms This commit expands the testing matrix for the C API tests to cover all the supported tier 1 platforms. As tier 1 platforms we document that we test them all on every PR, historically that's just been the Python testing, but as we grow the capabilities of our C API in Qiskit we should expand that guarantee to testing on all the tier 1 platforms too. There will need to be some iteration done to ensure they all work as expected, it's unlikely that this will work by default on Windows and we can tweak the configuration based on the results of CI runs. * Add complex number support for MSVC * Use `uint_X` over `u_int_X` for MSVC support * try making tests MSVC compat * aaand another run! now use a macro to define MSVC custom complex number constructor * forgot header guard * missed some complex, try fix zero-length array * msvc doesn't support equaliy on complex ... of course... * use f(void) over f() * Split out windows tests to separate command * all of the verbose! * Attempt 918263312 to fix windows life is pain Co-authored-by: Max Rossmannek <oss@zurich.ibm.com> Co-authored-by: Almudena Carrera Vazquez <almudenacarreravazquez@hotmail.com> * try using CMPLX * Revert "try using CMPLX" This reverts commit ac4e38d. * Deduplicate job definitions * store pyo3 build config during compilation maybe this should be set behind a flag that we enable if we detect we're running on windows, to avoid the dependency on unix * rm redundant import, also store name --------- Co-authored-by: Julien Gacon <jules.gacon@googlemail.com> Co-authored-by: Max Rossmannek <oss@zurich.ibm.com> Co-authored-by: Almudena Carrera Vazquez <almudenacarreravazquez@hotmail.com>
* fix result.to_dict() following new 2.0 typing * Update qiskit/result/result.py Co-authored-by: Luciano Bello <766693+1ucian0@users.noreply.github.com> * add result to_dict test * add release note * Update type hints to reflect current use of Result. We no longer have any ExperimentHeader class to call on, so the type can only be dict. * Remove reno, as the bug is technically unreleased --------- Co-authored-by: valente <valente@ar.ibm.com> Co-authored-by: Luciano Bello <766693+1ucian0@users.noreply.github.com> Co-authored-by: Elena Peña Tapia <epenatap@gmail.com>
* move cbindgen to Rust build * add include guard * allow cmdline build and compile-time build * write `qiskit.h` to `crates/cext` and use Makefile to correctly move or regenerate the file lazily * try cleaner build setup write header into target/qiskit.h and copy into dist/c/include with Makefile. The cargo build script tracks changes in target/qiskit.h and rebuilds if changed/deleted Not sure if this is the best way but it seems to work :) * try fix build deps, and fix clippy * fix merge artifact this is probably why CI doesn't work, we need to ensure the cdylib is built * include dir creation as dependency
…nd dag.unit (Qiskit#14133) * Filter Rust deprecation warning when calling dag.duration/dag.unit internally Filter Rust deprecation warning when calling dag.duration/dag.unit internally * Apply Matt's suggestions: * Refactor access patterns: add dag._duration and dag._unit * Refactor internal uses of dag.duration and dag.unit to rely on internal methods * Remove blanket warning filters from unit test config On top of these: * Extend deprecation warnings to dag.duration and dag.unit setters (previously only in getters) * Clean up unit test config from old filters * Change stacklevel * Fix circuit_to_dag * Handle failing tests * _DAGDependencyV2 is private and not a DAGCircuit, so don't use internal attributes * Reduce duplication in getters and setters
…14145) Fix the deprecation warning check to use the correct `assertWarns()` method rather than `assertRaises()` that is used to test for exceptions. This fixes the test suite when it is run without `-Werror`.
This commit bumps the PyO3 version we use in Qiskit to the latest pyo3 release 0.24.1. Luckily this time there don't seem to be any API changes required to upgrade so it's a straight version bump.
…oup (Qiskit#14135) Bumps the github_actions group with 1 update: [pypa/cibuildwheel](https://github.com/pypa/cibuildwheel). Updates `pypa/cibuildwheel` from 2.23.1 to 2.23.2 - [Release notes](https://github.com/pypa/cibuildwheel/releases) - [Changelog](https://github.com/pypa/cibuildwheel/blob/main/docs/changelog.md) - [Commits](pypa/cibuildwheel@v2.23.1...v2.23.2) --- updated-dependencies: - dependency-name: pypa/cibuildwheel dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github_actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [indexmap](https://github.com/indexmap-rs/indexmap) from 2.7.1 to 2.8.0. - [Changelog](https://github.com/indexmap-rs/indexmap/blob/main/RELEASES.md) - [Commits](indexmap-rs/indexmap@2.7.1...2.8.0) --- updated-dependencies: - dependency-name: indexmap dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Modified the dump method to include byte offsets in QPY header * Updated interface.py * Updated interface.py * Serial dump and load with byte offsets + Code refactoring * Refactored to use hard-coded version for QPY version 16+ support * Updated interface.py * Updated interface.py * Added support for circuit table header with byte offsets in QPY * Added release notes * Updated format specification in __init__.py * Added named tuple for CIRCUIT_TABLE_ENTRY * Updated dump to seek to end of file after writing byte offsets * Updated __init__.py * Use formats for writing the offsets. Co-authored-by: Kevin Hartman <kevin@hart.mn> * Update __init__.py Co-authored-by: Kevin Hartman <kevin@hart.mn> * Fixed linting * Updated __init__.py to explicitly describe older formats. Co-authored-by: Kevin Hartman <kevin@hart.mn> * Removed redundant description from __init__.py Co-authored-by: Kevin Hartman <kevin@hart.mn> * Update __init__.py to capitalize "Rust" Co-authored-by: Kevin Hartman <kevin@hart.mn> * Update release notes to label this change as a feature rather than an update. Co-authored-by: Kevin Hartman <kevin@hart.mn> * Update interface.py to use offset in the named tuple of circuit table entry (instead on [0]) Co-authored-by: Kevin Hartman <kevin@hart.mn> * Update interface.py * Fixed indentation error --------- Co-authored-by: Kevin Hartman <kevin@hart.mn>
…iskit#14660) This commit updates the internal construction of the SymbolExpr struct use a reference counting pointer instead of a regular pointer to a heap allocated object. The way the binary tree gets constructed internally using a normal pointer results in a lot of a copies of elements on the tree. This creates a large performance overhead for the symbol expr as we end up copying and freeing elements on the binary tree a large amount. This was the root cause of the regression reported in Qiskit#14653 as the expression that gets generated by the transpiler internally ends up adding a lot of elements to global phase expression and that results in a lot of memory allocation and deallocation overhead. By using a reference counting pointer instead of creating and freeing copies of the expressions on the tree we instead only keep a single copy and just increment or decrement the reference count. Fixes Qiskit#14653
Thank you for updating the heuristics. I compare the performance with this script. It gets closer to 2.0.3.
|
Bumps [indexmap](https://github.com/indexmap-rs/indexmap) from 2.9.0 to 2.10.0. - [Changelog](https://github.com/indexmap-rs/indexmap/blob/main/RELEASES.md) - [Commits](indexmap-rs/indexmap@2.9.0...2.10.0) --- updated-dependencies: - dependency-name: indexmap dependency-version: 2.10.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix future clippy issues This commit fixes some issues the clippy on nightly has identified as issues in the code. These will all become CI failures at some future date (after we raise the MSRV to match the current rules nightly is adding). However, most of the changes are good improvements and in this case most are code style issues that make things a bit more concise. The bulk of the changes here are just moving to use captured identifiers in format strings. * Update new qasm3 code too * Fix new issues introduced in new commits * Remove unneeded return statements * Make lifetime syntaxes consistent A new nightly rule was added to ensure that lifetime syntaxes are matched in function signatures. In many cases we were excluding the elided anonymous lifetime parameter being returned in the source which clippy asserts is potentially confusing. This commit adds the explicit anonymous paramter to these places to fix the warning. * Fix rust tests too
This reverts commit da1c655.
fa7af5e
to
5f1bc85
Compare
This reverts commit 5f1bc85.
I'm trying to solve reno UID issue, I have broken this repo. |
This reverts commit c64cd90.
I will open another PR for this fix |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR is an improvement of SymbolExpr
This is follow up PR to #14508
SymbolExpr has issue in calculating with integer division. This PR adds expression for integer fractional numbers by accepting
BinaryOp::Div
to have 2 integer values and this type is treated as similar toSymbolExpr::Value
to optimize the equationAnd this is follow up PR for issue #14653
Details and comments
Previously
a/3 + 4*a/3
was not optimized as5*a/3
, because4*a/3
was stored asBinary(Mul, Value(4), Binary(Div, Symbol("a"), Value(3)))
and it is too complicated to add to other node.In this PR,
4*a/3
is expressed byBinary(Mul, Binary(Div, Value(4), Value(3)), Symbol("a"))
it is much easier to find out this expression is multiply of a value and symbol.In #14508, I tried to implement
Value::Fraction
but I noticed I can implement the same optimization by usingBinary(Div, Value(), Value())
expression.In #14653, many non-symbolic parameters (values) are not added that increases the depth of tree of the equation. In this PR, some heuristic are added to add/sub values in the equation to decrease the memory usage and improve the speed