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

Cleanup PR #69

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

Cleanup PR #69

wants to merge 11 commits into from

Conversation

JonhasSC
Copy link
Collaborator

@JonhasSC JonhasSC commented Jan 10, 2025

This PR is focused on cleaning up a bit of #68 and #65. Estimate resources handles gate accuracy differently when it comes to performing cpt transformations or through quatran manipulation, which wasn't totally clear. This resulted in negative resources when using the default argument, 10. The intended usage was to express it as 1e-10, but pyLIQTR does not handle it that way.

On top of those bug fixes, does some QOL changes for the scripts.

value_per_t_gate is handled better as well

Copy link
Collaborator

@zain2864 zain2864 left a comment

Choose a reason for hiding this comment

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

Just a few comments, some functionality was already tested when doing RE's for Dynamics and GSEE, other than that, it looks good to me.


gen_json(logical_re, outfile, metadata )
return cpt_trotter
logical_re = estimate_cpt_resources(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, I did some testing with using Trotter for Dynamics, if use_analytical = False, but given both value and reps, the value_per_t_gate is recorded as null. Is this the correct usage?

@@ -28,6 +29,7 @@ class EstimateMetaData:
is_extrapolated: bool=field(default=False, kw_only=True)
gate_synth_accuracy: int | float = field(default=10,kw_only=True)
value_per_circuit: float | None=field(default=None, kw_only=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we keeping this as value_per_circuit, or just changing to value?

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