Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some abbreviations for gates and linear algebra #12651

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

jlapeyre
Copy link
Contributor

@jlapeyre jlapeyre commented Jun 25, 2024

Add constant abbreviations for some values and types.

This PR introduces some abbreviations for repetitive Rust code. Motivations are reducing clutter, improving readability, and perhaps modest support for rapid development.

Construction of complex numbers

Complex numbers are common in scientific computing and ubiquitous in quantum computation. They are supported in Rust via libraries, one in particular, with few ergonomic features beyond those provided for any struct. We do have Complex64 for Complex<f64> so that numbers are constructed with

Complex64::new(a, b)

In other scientific languages and platforms, developers typically appreciate ergonomic features1 reflecting special status of complex numbers. Improving ergonomic for complex numbers in Qiskit is a worthwhile task.

In this PR, I chose to continue using the definition of const fn 64 that was introduced in #12459

#[inline(always)]
const fn c64(re: f64, im: f64) -> Complex64 {
Complex64::new(re, im)
}
. I have moved the definition to a more neutral location and used it uniformly in all crates.

I chose this in part to try to make this appealing to Qiskit devs. However there are choices.

  1. The current c64 (referenced above). Can be used in static and const constructs. Requires f64 arguments.
  2. c64 introduced recently in num-complex. This creates a new Complex<f64> from arguments that can convert Into<f64>. For example c64(1.0, 2.0) and c64(1, 2). static and const not supported, which is a major disadvantage for Qiskit.
  3. I prefer defining c64 slightly more generally so that c64(1, 2.0) is allowed (The version in item 2. does not support this.). This is a minor tweak to the implementation. However, static and const are still not supported.
  4. A macro c64! is the most capable and flexible. This is functionally nearly the same as item 3. with the major advantage that static and const are supported.

One might argue that having c64(1, 2) (or c64!(1, 2)) return a Complex64 goes against the spirit of Rust. But I find decimal points that exist only to satisfy language semantics irksome and distracting. For rapid development, allowing more flexibility within constraints of semantics is useful. It would be better that c64(1, 2) construct a complex number with integer types in combination with facilities mix complex types. But Rust in general is too explicit for this.

However, examining this PR, you can see that the number of superfluous decimal points is actually rather small at least so far. So retaining choice 1 is not a big burden.

Constants for common complex and f64 values

These are const bindings for very common complex values, such as C_ONE, C_ZERO etc. I think just ONE and ZERO are just as good, a bit less explicit, but neater and more readable (@sbrandhsn). These make source more readable and less cluttered, and likely less error prone. A couple of common f64 values that are defined repeatedly locally have been moved to a common location. (@ShellyGarion @alexanderivrii)

Type definitions for arrays representing gates

For example GateArray1Q = [[Complex64; 2]; 2];. Advantages are reducing clutter, signalling intent, etc.

One could object to this and the previous item. These are less explicit and one might mistakenly assume that an identifier is bound to a different value. But these are generic objections to binding an identifier to a value. The statements of the objections are always true. Yet we find these refactorings sometimes worthwhile.

Relation to other PRs

The other part of #12507, which implements a gate, will be included in a separate PR.

Footnotes

  1. In Python the suffix j is recognized by the parser, so 2j represents the complex value (0.0, 2.0).

    In Julia, Complex{T<:Real} is a parametric type and the constant im is bound to the value Complex{Bool}(false, true). The parser reads a numeric literal followed by an identifier as a call to the function *. Methods are defined for arithmetic with mixed types, so that 2.0im dispatches to *(x::Real, z::Complex{Bool}) = Complex(x * real(z), x * imag(z)).

    Technical computing is not a core domain of C. However concessions to ergonomics exist. In C99 a complex number is typically constructed like this: 1.0 + 2.0*I. In C11, CMPLX(1.0, 2.0).

    Fortran has special support for complex numbers, since it's main application domain is scientific/technical computing. For example z%re = 3.0 assigns the real part of z.

@coveralls

This comment was marked as off-topic.

@jlapeyre jlapeyre marked this pull request as ready for review June 25, 2024 19:39
@jlapeyre jlapeyre requested a review from a team as a code owner June 25, 2024 19:39
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core
  • @kevinhartman
  • @levbishop
  • @mtreinish

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9668651215

Details

  • 96 of 96 (100.0%) changed or added relevant lines in 9 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.006%) to 89.726%

Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/sparse_pauli_op.rs 1 93.27%
crates/qasm2/src/lex.rs 5 92.11%
Totals Coverage Status
Change from base Build 9661630219: 0.006%
Covered Lines: 63725
Relevant Lines: 71022

💛 - Coveralls

@jlapeyre
Copy link
Contributor Author

In an out of band discussion we agreed to reduce use of the constants PI2 and PI4. In particular, the commit above that makes them pub const has been overwritten to remove these in b6b315e .

The commit message for b6b315e is written as if the pub const definitions never existed. So the message can be preserved as is in a squash commit. But the message for the commit introducing pub const PI2 etc. should be removed when squashing.

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9700404596

Details

  • 112 of 115 (97.39%) changed or added relevant lines in 10 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.002%) to 89.76%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/operations.rs 15 18 83.33%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/sparse_pauli_op.rs 1 93.27%
crates/qasm2/src/lex.rs 4 92.88%
Totals Coverage Status
Change from base Build 9698604147: 0.002%
Covered Lines: 63812
Relevant Lines: 71092

💛 - Coveralls

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

This looks fine to me and inline with what we discussed. If you want to squash-rebase / change your commit messages or anything, feel free to and I'll re-approve. If you don't mind, just tag it for merge.

@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository labels Jun 27, 2024
This PR introduces some abbreviations for repetitive Rust code. Motivations are reducing
clutter, improving readability, and perhaps modest support for rapid development.

* Use the definition of `const fn 64` that was introduced in Qiskit#12459 uniformly in all crates.

* Define some complex constants `C_ONE`, `C_ZERO`, `IM`, etc.

* Introduce type definitions for arrays representing gates. For example:
     `GateArray1Q = [[Complex64; 2]; 2];`
@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9701734273

Details

  • 112 of 115 (97.39%) changed or added relevant lines in 10 files are covered.
  • 16 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 89.743%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/operations.rs 15 18 83.33%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/sparse_pauli_op.rs 1 93.27%
crates/qasm2/src/lex.rs 3 92.88%
crates/qasm2/src/parse.rs 12 96.69%
Totals Coverage Status
Change from base Build 9698604147: -0.02%
Covered Lines: 63800
Relevant Lines: 71092

💛 - Coveralls

@jlapeyre jlapeyre added this pull request to the merge queue Jun 27, 2024
Merged via the queue into Qiskit:main with commit ea5a54b Jun 27, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants