Skip to content

Conversation

doichanj
Copy link
Collaborator

@doichanj doichanj commented Jun 6, 2025

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 to SymbolExpr::Value to optimize the equation

And this is follow up PR for issue #14653

Details and comments

Previously a/3 + 4*a/3 was not optimized as 5*a/3, because 4*a/3 was stored as Binary(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 by Binary(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 using Binary(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

radams78 and others added 30 commits March 21, 2025 17:42
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>
velocityraptor7085 and others added 5 commits June 24, 2025 18:37
* 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
@doichanj doichanj changed the title Add optimizations for integer fractional numbers in SymbolExpr Add heuristic to SymbolExpr Jun 27, 2025
@t-imamichi
Copy link
Member

Thank you for updating the heuristics. I compare the performance with this script. It gets closer to 2.0.3.

qiskit_version=2.0.3
2.205333708989201 sec
# this PR
qiskit_version=2.2.0.dev0+986d128
2.718509083002573 sec
# main branch
qiskit_version=2.2.0.dev0+8353251
4.531503707999946 sec

dependabot bot and others added 7 commits June 27, 2025 11:38
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
@doichanj doichanj changed the title Add heuristic to SymbolExpr Add more heuristic to SymbolExpr Jul 2, 2025
@doichanj doichanj requested review from a team and jyu00 as code owners July 2, 2025 04:35
@doichanj doichanj force-pushed the symexpr_add_fractional_opt branch from fa7af5e to 5f1bc85 Compare July 2, 2025 11:05
@doichanj doichanj requested a review from nonhermitian as a code owner July 2, 2025 11:05
@doichanj
Copy link
Collaborator Author

doichanj commented Jul 2, 2025

I'm trying to solve reno UID issue, I have broken this repo.
I'm sorry to send unnecessary review requests (I do not know why these requests are sent)
Is it better to close this PR and reopen?

@doichanj
Copy link
Collaborator Author

doichanj commented Jul 2, 2025

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.