Skip to content

Conversation

@tungbuidang
Copy link
Collaborator

I added some basic visualization method to the 3 data structures assigned. Since I print them in the terminal, the editing is minimal and I made some deliberate choice to hard code decimal points to keep the formatting consistent. I would need some comment on how to improve the code for more readability and flexibility.

Copy link
Collaborator

@Ectras Ectras left a comment

Choose a reason for hiding this comment

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

I like the visualization, maybe someone else can give their opinion as well. My comments are mostly about code quality. The comments are intentionally nitpicky, to help learn idiomatic Rust :)


for i in 0..self.size() {
print!("QB{} ||", i + 1);
let column = self.pauli_column(i).to_string();
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my opinion, it's a bit risky to rely on the string representation of PauliString. I'd rather construct the string here from the ground up, using e.g. pauli_letter to get the letters in the Pauli string.

@Ectras
Copy link
Collaborator

Ectras commented Aug 22, 2025

Actually, I find the headers of the Clifford tableau output a bit confusing. I.e., I'm talking about the X1 Z1| X2 Z2 ... here:

    || X1 Z1| X2 Z2| X3 Z3| X4 Z4|
+/- || +  + | +  + | +  + | +  + |
QB0 || X  I | I  I | Z  I | I  I | 
QB1 || I  X | I  I | I  Z | I  I | 
QB2 || I  I | X  I | I  I | Z  I | 
QB3 || I  I | I  X | I  I | I  Z |

It looks like the columns of a normal Clifford tableau. But we print the columns as rows, i.e., the whole thing is transposed. I don't think the headers make sense then. The first n columns are destabilizers and the other n are stabilizers. The column dividers then don't match either. There should be a single divier after n columns, I think.

    || destab.    | stab.      |
+/- || +  +  +  + | +  +  +  + |
QB0 || X  I  I  I | Z  I  I  I | 
QB1 || I  X  I  I | I  Z  I  I | 
QB2 || I  I  X  I | I  I  Z  I | 
QB3 || I  I  I  X | I  I  I  Z |

@tungbuidang
Copy link
Collaborator Author

A general comment on this task is I am not so sure on how do I use PyO3 to port these display functionality to python ? Since I have already implement the Display trait on all 3 data structures, in python the user should be able to just print the object (clifford tableaus, pauli polynomial) constructed directly? Is the constructor for these object in Python created/binded yet ?

Comment on lines 10 to 11
// Stab: ZZZ, -YIY, XIX
// Destab: -IXI, XXI, IYY
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these stabilizers and destabilizers correspond ot the tableau you are creating in the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fixed it in the latest commit

Comment on lines 33 to 36
let ham = vec![("IZZZ", 0.3)];
let pp = PauliPolynomial::from_hamiltonian(ham);
let ct = CliffordTableau::new(4);
let pe = PauliExponential::new(VecDeque::from([pp]), ct);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can reuse the pp and tableau from before

Comment on lines 39 to 45
// //visualize_pauli_exponential complex
let ham = vec![("IXYZ", 0.3), ("XXII", 0.7), ("YYII", 0.12)];

let pauli_polynomial = PauliPolynomial::from_hamiltonian(ham);
let clifford_tableau = CliffordTableau::new(4);
let complex_pe = PauliExponential::new(VecDeque::from([pauli_polynomial]), clifford_tableau);
println!("{}", complex_pe);
Copy link
Contributor

Choose a reason for hiding this comment

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

then this would be redundant

Comment on lines 328 to 334
for (i, column) in self.pauli_columns.iter().enumerate() {
write!(f, "QB{} ||", i)?;
let mut out = String::new();
let mut letter_count = 0;
for j in 0..column.len() {
// let letter = column.pauli(j);
match column.pauli(j) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you are printing the tableau transposed. i.e. the columns are printed as rows.

Comment on lines 1196 to 1242
ct.to_string(),
"CliffordTableau(3)\nZ Y I I Z Z\nZ I X X I I\nZ Y Y I I Z\n+ - + - + +"
" || X1 Z1| X2 Z2| X3 Z3|\n+/- || + - | + - | + + |\nQB0 || Z Y | I I | Z Z | \nQB1 || Z I | X X | I I | \nQB2 || Z Y | Y I | I Z | \n\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you are printing the transpose, which was the limitation of the debug formatter because it's easy to print the columns are rows.
Something like this:

"    || Stabilizers | Destabilizers |\n QB0 || + Z Z Z | - I X I | \nQB1 || - Y I Y | + Z I I | \nQB2 || + I X Y | + Z I Z | \n\n"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should be correct now

let pp = setup_sample_pp();
assert_eq!(
pp.to_string(),
"Angles | 0.300 | 0.700 | 0.120 |\nQubit 0| I | X | Y |\nQubit 1| Z | Y | X |\nQubit 2| Y | I | X |\n\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Be more consistent with the formatting wrt the tableau. "QB0" vs "Qubit 0".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -29,6 +30,17 @@ impl PauliExponential {
}
}

impl fmt::Display for PauliExponential {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing test.
It would be cool if you can make it look something like this:

    || 0.2 | 0.4 || 0.12 || Stabilizers | Destabilizers ||
QB0 || X   | I   || Z    || + Z Z Z     | - I X I       || 
QB1 || I   | I   || Z    || - Y I Y     | + Z I I       ||
QB2 || I   | Z   || Z    || + I X Y     | + Z I Z       ||

Where the || separate the pauli polynomials in the pauli exponential, followed by the stabilizers and destabilizers

Copy link
Contributor

Choose a reason for hiding this comment

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

This would require a refactor of the other formatters so that they can give partial solutions per line that you can glue together.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@Aerylia
Copy link
Contributor

Aerylia commented Sep 1, 2025

@tungbuidang

task is I am not so sure on how do I use PyO3 to port these display functionality to python ?

Looks like you'll find how to do that here: https://stackoverflow.com/questions/62666926/str-function-of-class-ported-from-rust-to-python-using-pyo3-doesnt-get-used

@Ectras Is the stackoverflow a good source?

@Ectras
Copy link
Collaborator

Ectras commented Sep 10, 2025

@tungbuidang

task is I am not so sure on how do I use PyO3 to port these display functionality to python ?

Looks like you'll find how to do that here: https://stackoverflow.com/questions/62666926/str-function-of-class-ported-from-rust-to-python-using-pyo3-doesnt-get-used

@Ectras Is the stackoverflow a good source?

I don't think it's necessary (anymore) to implement a specific trait, just name the functions the same as in Python. See also https://pyo3.rs/v0.15.1/class/protocols.

Here is a small example:

use std::fmt;

use pyo3::prelude::*;

struct RustType {
    num: i32,
}

impl fmt::Display for RustType {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Rust says {}", self.num)
    }
}

#[pyclass]
struct Wrapper(RustType);

#[pymethods]
impl Wrapper {
    #[new]
    fn new(num: i32) -> Self {
        Wrapper(RustType { num })
    }

    // This method is recognized as magic function by pyo3
    fn __str__(&self) -> PyResult<String> {
        // to_string() uses the Display trait
        Ok(self.0.to_string())
    }
}

@tungbuidang
Copy link
Collaborator Author

I am pretty happy with the Rust print function at the moment, but I still struggle with PyO3. from @Ectras comment it seems that I only need to define str for each object type ? But I have not seen the PauliPolynomial and PauliExponential implementation in the synpy folder.

what would probably help me to move forward is may be a demonstration of how to write a Wrapper for the PP and PE, bind it, and a small python example of object instantiation and usage. I checked out @Aerylia pull request on the user guide to find some inspiration but it seems that she use pauliopt library instead ?

@tungbuidang
Copy link
Collaborator Author

image

The printing should look like this now

Some of the things that I try to incorporate:

  • support up until qubit 99 alignment (qubit 10 onward there is an additional char that messed up the format)
  • support long and short stabilizers / destabilizers length
  • Each Pauli Polynomial in Pauli Exponential are separated by a "||"

Something that I have not done but I feel might be un-necessary

  • The alignment goes off if the maximum length of line is exceeded. Currently I have no way to split a long line to multiple lines and keep the formatting. However for such detailed data structure I am not sure if this printing is a good way to debug.

@Ectras
Copy link
Collaborator

Ectras commented Sep 15, 2025

I think the output looks nice now. Does printing "empty things" (where this makes sense) also work? Also, you could make the alignment independent of the number of qubits by using ilog10(num_qubits) to get the max width of numbers, e.g. 120 -> 3, 4 -> 1, 55 -> 2, 100 -> 3, ... and then padding the numbers accordingly.

I wouldn't worry about the case where the max line length of the output media (e.g. the terminal) is not sufficient. I don't know any tool that cares about this. If the users want large output without any text wrapping, they should pipe the output to a file.

@Aerylia
Copy link
Contributor

Aerylia commented Oct 7, 2025

Ah, I believe the part that has the X matrix is the Destabilizers and Z matrix is stabilizers because Z stabilizes the computational basis. X|0> = |1> and Z|0> = |0>

@tungbuidang
Copy link
Collaborator Author

Ah, I believe the part that has the X matrix is the Destabilizers and Z matrix is stabilizers because Z stabilizes the computational basis. X|0> = |1> and Z|0> = |0>

I checked the code, apparently the small example print correctly according to the comment left by previous developer. For the big example, X and Z was called using 'CliffordTableau::new'. I probably need a bit more discussion on this

@Aerylia
Copy link
Contributor

Aerylia commented Oct 22, 2025

On the task of printing empty things:

  • Print things with 0 qubits should not panic
  • Print things with empty vecs should print something resembling the empty vecs
  • PauliExponential with empty vec != PauliExponential with 1 PauliPolynomial with Empty vec => Printing should reflect that.

@tungbuidang
Copy link
Collaborator Author

latest commit include the handling of "empty" printing. They should look like this now:
image

A few comments:

  • Panic did not happen previously on "printing" function. It happens when we construct a pauli polynomial from an empty string. In fact I was not able to produce an empty pauli polynomial to test the print, and I added a function to create "empty" pauli polynomial.

  • "print empty" now only check if the size of the 'vec' is zero. The printing does not handle cases where 'vec' size is not match to 'size' of PauliPolynomial. Doing this will probably produce panic.

  • comment on the display is welcome !

@tungbuidang tungbuidang requested a review from Aerylia October 24, 2025 08:59
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.

4 participants