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

Add iqm fake aphrodite #113

Merged
merged 6 commits into from
Aug 28, 2024
Merged

Conversation

JMuff22
Copy link
Contributor

@JMuff22 JMuff22 commented Aug 14, 2024

Hi, this pull request adds IQMFakeAphrodite to provide noisy simulation of IQM's Aphrodite chip. I created the changes based off IQMFakeApollo. The values are currently placeholder values which are open to be changed by the maintainers. They are based off #66.

Should any documentation be added regarding this PR? Let me know and I will add it.

Additionally, I added 1 comment to CONTRIBUTING.rst to inform contributors that they can run the formatting through tox.

Note that the tests will fail due to duplicate code as the values are copied from IQM fake apollo

Copy link
Contributor

@Aerylia Aerylia left a comment

Choose a reason for hiding this comment

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

You'll have to fix the linting issues for tests to pass.

If you can rename the noise model name, before merging, that would be nice. But the other fake backends don't have a sensible name either, so it's not a big deal.

Comment on lines +22 to +35
def test_iqm_fake_aphrodite():
backend = IQMFakeAphrodite()
assert backend.num_qubits == 54
assert backend.name == 'IQMFakeAphroditeBackend'


def test_iqm_fake_aphrodite_connectivity(aphrodite_coupling_map):
backend = IQMFakeAphrodite()
assert set(backend.coupling_map.get_edges()) == aphrodite_coupling_map


def test_iqm_fake_aphrodite_noise_model_instantiated():
backend = IQMFakeAphrodite()
assert isinstance(backend.noise_model, NoiseModel)
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests mainly check if the python interpreter works, but they are in line with the existing tests for FakeBackends. It's good for now, but I'll make a ticket to improve FakeBackend testing.

src/iqm/qiskit_iqm/fake_backends/fake_aphrodite.py Outdated Show resolved Hide resolved
@JMuff22
Copy link
Contributor Author

JMuff22 commented Aug 23, 2024

You'll have to fix the linting issues for tests to pass.

If you can rename the noise model name, before merging, that would be nice. But the other fake backends don't have a sensible name either, so it's not a big deal.

@Aerylia It was my understanding that the CI test fails due to "duplicate code" between the noise model for aphrodite and apollo. This is because the values themselves are copy and pasted. I am not sure how to fix this issue other than use different values. The values are placeholders anyway if we follow the convention from #66

@Aerylia
Copy link
Contributor

Aerylia commented Aug 28, 2024

For next time: The duplicate code linting issue can be fixed with # pylint: disable=duplicate-code. In the future, we want a better format generating these fake devices, but since that doesn't exist yet, we are stuck with this.

I've made the changes and updated the branch with main. So it's ready to merge when checks pass.

@Aerylia Aerylia merged commit 9f9689e into iqm-finland:main Aug 28, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants