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

Remove Backend.characterisation #226

Merged
merged 4 commits into from
Feb 17, 2022
Merged

Conversation

cqc-alec
Copy link
Collaborator

After merging this I'll make a pytket pre-release and then update the extensions.

@cqc-alec cqc-alec requested a review from sjdilkes February 16, 2022 16:30
Copy link
Contributor

@sjdilkes sjdilkes left a comment

Choose a reason for hiding this comment

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

One query.

The circuit module provides an API to interact with the tket
:py:class:`Characterisation` suite, with methods for noise shaping, characterisation and
mitigation. This module is provided in binary form during the PyPI installation.
The `tailoring` module provides an API to interact with tket's charcterisation suite,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this isn't the PR for it, but faithfully the tailoring module only provides an API for using frame randomisation methods, and maybe we should rename the whole module to frame randomisation in another PR?

If we save that for another review, it should be tket's characterisation suite.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I'll revert this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. (Just me being lazy, I saw it was referring to the wrong module name and thought I'd correct it at the same time. But yes it should be a new PR.)

@cqc-alec cqc-alec requested a review from sjdilkes February 16, 2022 16:41
@ss2165
Copy link
Member

ss2165 commented Feb 16, 2022

After merging this I'll make a pytket pre-release and then update the extensions.

The changes in #214 will also break things in extensions, should the fixes be sister PRs?

@cqc-alec
Copy link
Collaborator Author

After merging this I'll make a pytket pre-release and then update the extensions.

The changes in #214 will also break things in extensions, should the fixes be sister PRs?

Good point. Maybe we should do a pre-release first without this change, fix those, and then do another pre-release with this change?

@cqc-alec
Copy link
Collaborator Author

cqc-alec commented Feb 17, 2022

After merging this I'll make a pytket pre-release and then update the extensions.

The changes in #214 will also break things in extensions, should the fixes be sister PRs?

Good point. Maybe we should do a pre-release first without this change, fix those, and then do another pre-release with this change?

Actually, removing this doesn't break the extensions, so I'm going to merge and attempt another pre-release now.

(For some reason I'm having difficulty doing a pre-release. Seems the wrong version number is being applied by setuptools_scm on some platforms...)

@cqc-alec cqc-alec merged commit 8691843 into develop Feb 17, 2022
@cqc-alec cqc-alec deleted the feature/nocharacterisation branch February 17, 2022 06:19
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.

3 participants