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

fix: Add parameter to decide wheter remove resource in aggregation function #109

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

sitong1011
Copy link
Contributor

@sitong1011 sitong1011 commented Aug 15, 2024

Description

This PR solves issue #106 by adding the remove_decomposed parameter to the add_aggregated_resources function.

Context:

In certain use cases, it is necessary to preserve the original resources after they have been aggregated. Previously, the add_aggregated_resources function automatically removed the original resources after aggregation.

Implemented Solution:

  • A boolean parameter remove_decomposed has been added to the add_aggregated_resources function.
    • If remove_original=True (default): The original resource is removed after aggregation.
    • If remove_original=False: The original resource is retained, and its type is marked as "other" to distinguish it from other resources (a straightforward approach; however, there may be more optimized solutions.)
  • Test cases have been added to validate the correct functioning of this new parameter in both scenarios (True and False).

Dependencies:

No new dependencies were added in this PR.

Please verify that you have completed the following steps

  • I have self-reviewed my code.
  • I have included test cases validating the introduced feature.
  • I have updated documentation.

@sitong1011 sitong1011 requested a review from mstechly August 15, 2024 23:20
@sitong1011 sitong1011 marked this pull request as ready for review August 15, 2024 23:20
@@ -37,6 +39,10 @@ def add_aggregated_resources(routine: Routine, aggregation_dict: Dict[str, Dict[
"arbitrary_z": {"T_gates": "3*log2(1/epsilon) + O(log(log(1/epsilon)))"},
...
}
remove_decomposed : Whether to remove the decomposed resources from the routine.
Copy link
Contributor

Choose a reason for hiding this comment

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

[nitpick] There's unnecessary space before :

@sitong1011 sitong1011 merged commit 37acec8 into main Aug 16, 2024
8 checks passed
@mstechly mstechly deleted the 106_aggregation branch August 26, 2024 20:07
This was referenced Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add a parameter in aggregation function to decide whether to remove the original resources
2 participants