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

Implement term handling & docs #36

Merged
merged 10 commits into from
Dec 20, 2024
Merged

Implement term handling & docs #36

merged 10 commits into from
Dec 20, 2024

Conversation

Cryoris
Copy link
Owner

@Cryoris Cryoris commented Dec 20, 2024

Summary

  • Add SparseTerm
  • Add PauliTermVec, IndexVec, BitTermVec
  • Improve tests
  • Add doxygen docs consistently

Copy link
Collaborator

@mrossinek mrossinek left a comment

Choose a reason for hiding this comment

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

A more thorough docs review can follow later

@@ -395,6 +395,402 @@ fn make_py_bit_term(py: Python) -> PyResult<Py<PyType>> {
Ok(obj.downcast_into::<PyType>()?.unbind())
}

/// An observable over Pauli bases that stores its data in a qubit-sparse format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we remove this docstring from the pure Rust version? Or do you want it duplicated entirely?

Copy link
Owner Author

Choose a reason for hiding this comment

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

We should strip it and adjust it to Rust. It currently contains a lot of Python-specific statements.

Comment on lines +39 to +40
indices: Vec<u32>,
bit_terms: Vec<BitTerm>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
indices: Vec<u32>,
bit_terms: Vec<BitTerm>,
indices: IndexVec,
bit_terms: BitTermVec,

Or do you see a reason not to do this?

Copy link
Owner Author

Choose a reason for hiding this comment

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

IndexVec & BitTermVec are removed now

///
#[no_mangle]
#[cfg(feature = "cbinding")]
pub extern "C" fn pauli_deallocate(pauli: &mut PauliTerm) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub extern "C" fn pauli_deallocate(pauli: &mut PauliTerm) {
pub extern "C" fn pauli_free(pauli: &mut PauliTerm) {

I recall you mentioned wanting to rename these functions like so, no?

num_failed += RUN_TEST(test_mult_real);
num_failed += RUN_TEST(test_mult_complex);
num_failed += RUN_TEST(test_mult);
fflush(stderr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you accidentally moved this line up

Suggested change
fflush(stderr);

Copy link
Owner Author

Choose a reason for hiding this comment

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

yeah I was trying to get C's printf to print interleaved with calls to Rust's println but that didn't work... I'll revert 🙂

num_failed += RUN_TEST(test_canonicalize);
num_failed += RUN_TEST(test_copy);
num_failed += RUN_TEST(test_num_terms);
num_failed += RUN_TEST(test_num_qubits);
num_failed += RUN_TEST(test_custom_build);
num_failed += RUN_TEST(test_term);

fprintf(stderr, "=== Number of failed subtests: %i\n", num_failed);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fprintf(stderr, "=== Number of failed subtests: %i\n", num_failed);
fprintf(stderr, "=== Number of failed subtests: %i\n", num_failed);
fflush(stderr);

@Cryoris Cryoris merged commit 74e9fda into c-sparse-observable Dec 20, 2024
6 checks passed
@Cryoris Cryoris deleted the c-api branch December 20, 2024 14:12
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.

2 participants