Qiskit-terra: Convert Hamiltonian variables to Hz

[taalexander]

Information

• Qiskit Terra version:
• Python version:
• Operating system:

What is the current behavior?

All other time-based units are currently defined in terms of seconds/Hertz. The Hamiltonian variables should also be. This may have an effect on the Qiskit-Aer pulse simulator @chriseclectic @DanPuzzuoli.

Steps to reproduce the problem

What is the expected behavior?

Suggested solutions

[eembees]

Is this still open or will it be solved by #3597 ?

[taalexander]

This is still open. Are you interested in taking it on?

[eembees]

I'll get to work and update you when I have something to show :)

[taalexander]

Thank you very much!

[ToJen]

Is this still open and what are some suggested modules to look at in order to fix this?

[eembees]

Is this still open and what are some suggested modules to look at in order to fix this?

Yes, this is still open @ToJen - It fell a bit out of focus for me, so I don't have anything complete yet. You're very welcome to take this over, if you want to!
I think the modules you should start by looking at are the qiskit.providers.models.backendconfiguration, specifically the PulseHamiltonianSchema class.

[taalexander]

Thanks for letting us know @eembees, I've unassigned you and this issue is now open for someone else to take on.

[ramyasuresh123456]

Is this still open? I can take it on if so.

[arunraja-hub]

@taalexander I am keen to contribute to solving this as one of my first issues. Can I work on this?
If possible could you share the steps to reproduce this error? Thank you

[taalexander]

Hello @arunraja-hub, I believe this is still unassigned. To get started I would look at @eembees comment above. This would be similar to the approach taken with PulseDefaults. Basically make the supplied hamiltonian variables private and expose a property with the converted values.

[arunraja-hub]

There is no PulseHamiltonianSchema class in qiskit/providers/models/backendconfiguration.py

[taalexander]

You're right. There was a relatively major change to providers since @eembees comment. Please see https://github.com/Qiskit/qiskit-terra/blob/fab61d21d105c40142e7d67a8ea7bab602dd4c12/qiskit/providers/models/backendconfiguration.py#L485 for the hamiltonian attribute. The hamiltonian parameter could be reformatted before setting the hamiltonian attribute. See https://github.com/Qiskit/qiskit-terra/blob/fab61d21d105c40142e7d67a8ea7bab602dd4c12/qiskit/providers/models/backendconfiguration.py#L479 for an example.

[arunraja-hub]

From my understanding, hamiltonian is currently defined in terms of GHz as seen in https://github.com/Qiskit/qiskit-terra/blob/fab61d21d105c40142e7d67a8ea7bab602dd4c12/qiskit/providers/models/backendconfiguration.py#L479
I will have to remove 1e9 to represent the hamiltonian in Hz , correct?

[taalexander]

The Hamiltonian comes from the backend in GHz, so you will have to add a factor 1e9, to each entry in the vars section to convert to Hz.

[arunraja-hub]

There is no vars in qiskit-terra/qiskit/providers/models/backendconfiguration.py. Should I be looking for it in another file?
Also, sorry if I am asking the obvious here, according to https://github.com/Qiskit/qiskit-terra/blob/fab61d21d105c40142e7d67a8ea7bab602dd4c12/qiskit/providers/models/backendconfiguration.py#L479
aren't we already multiplying the ranges by 1e9?

[arunraja-hub]

@taalexander a gentle reminder to help with the issue in the message above
Thank you very much!

[taalexander]

My apologies for the delay in getting back to you @arunraja-hub, I do not believe there is a Hamiltonian object, but it arrives with a structure as described in line 158 here https://www.github.com/Qiskit/qiskit-terra/tree/master/qiskit%2Fschemas%2Fbackend_configuration_schema.json. if present it should be a dictionary of this structure. It is this vars field that should be modified.

See page 34 of https://arxiv.org/abs/1809.03452 for more info on the input structure.

[arunraja-hub]

Thank you for your reply, @taalexander

The hamiltonian is initialised here:

https://github.com/Qiskit/qiskit-terra/blob/fab61d21d105c40142e7d67a8ea7bab602dd4c12/qiskit/providers/models/backendconfiguration.py#L399

It is a dictionary of strings. So do I change to a dictionary of integers?

[taalexander]

This type hint seems to have been added recently and unfortunately is wrong. This will need to be corrected. I would encourage you to look at the hamiltonian for ibmq_armonk with backend.configuration().hamiltonian to see the structure that is actually present. This is also another example. https://github.com/Qiskit/qiskit-terra/blob/e265409a4551551feddb4a74928386fd0d27f6df/qiskit/schemas/examples/backend_configuration_openpulse_example.json#L40

[taalexander]

I've created #4649 to track this bug.

[taalexander]

@arunraja-hub, are the requirements a bit clearer now?

[arunraja-hub]

https://github.com/Qiskit/qiskit-terra/issues/4649 has been fixed now
the Hamiltonian specification in https://arxiv.org/pdf/1809.03452.pdf,

Do I multiply each entry in varsas follows?:
"vars" : {"v0": 5.0*1e9, "v1" : 5.25*1e9},

[taalexander]

4649 has been fixed now

the Hamiltonian specification in https://arxiv.org/pdf/1809.03452.pdf,

Do I multiply each entry in varsas follows?:
"vars" : {"v0": 5.0*1e9, "v1" : 5.25*1e9},

Yes :)

[arunraja-hub]

Since, do I edit the description of vars in the hamiltonian dictionary as follows? :
from
https://github.com/Qiskit/qiskit-terra/blob/35db627fbd1c9762d9f394c71a1df129c24797f4/qiskit/schemas/backend_configuration_schema.json#L23
to
"description": "Variables in the h_str. Multiply by 1e9 to convert from GHz to Hz"},

[taalexander]

If you want to modify the description to something like "description": "Variables in the h_str in GHz." this is fine. The description is just for human readers.

[arunraja-hub]

Noted. With regards to changing the code to reflect the conversion from GHz to Hz, I am not sure in which file to make this change at

[taalexander]

Please see my PR #4728, for an idea of how to handle this in the configuration.

[arunraja-hub]

@taalexander Thank you.
Do I multiply the values in vars here by 1e9?: https://github.com/Qiskit/qiskit-terra/blob/51b70a7c3a555573c38181c65f61c446dcfc7e90/qiskit/test/mock/utils/configurable_backend.py#L214

ANd can I develop the test here: https://github.com/Qiskit/qiskit-terra/blob/ff5fede2dadf338e7ba36a3efcbf4df6b0e651aa/test/python/mock/test_configurable_backend.py#L104

[taalexander]

I do not think you have to multiple in the first case since that is the transport representation and the conversion should happen in the initialization of the PulseBackendConfiguration. In the second code link you will have to modify to pass the test.

[menehune23]

@taalexander Hello, I'd be interested in looking into this if it's gone stale. I don't see an assignee currently. Let me know!

[arunraja-hub]

Hi @menehune23 I am still working on this. Will let you know if I hit a dead-end

[menehune23]

No problem @arunraja-hub. Don't wanna step on your toes. I've got a solution and corresponding test if you'd like any hints. Cheers :)

[arunraja-hub]

Thanks @menehune23
I changed the specification in the json file as follows since we converted the values to GHz:
_"description": "Variables in the h_str in Hz"}_

[menehune23]

@arunraja-hub Recall though that the backend returns values in GHz, and that JSON schema is for the backend. This issue is to convert them to Hz upon consumption, but leave the backend as is (GHz).

It might be confusing because we’re multiplying by 1e9, but if you think about it long enough, it should make sense (think about going from kilometers to meters: 1km == 1000m). In general, we multiply to go to smaller units, because there are more of them.

[menehune23]

Sorry @arunraja-hub, I didn’t mean for my solution branch to pop up here and give you my answer directly. I was simply saving my work. TIL that this happens even across forks ;)

That said, it wasn’t really intended to copy/paste from, so I’d recommend seeing if you have a potentially different/better way to implement the change, now that you know which files might be involved.

[menehune23]

Also, if I read the contribution guidelines correctly, you’ll want to add a release note. I’m assuming this is user-facing, though I’m not 100% sure yet.

[taalexander]

Also, if I read the contribution guidelines correctly, you’ll want to add a release note. I’m assuming this is user-facing, though I’m not 100% sure yet.

Good point @menehune23 this will be a userfacing change with potentially breaking impacts for the Aer pulse simulator

[menehune23]

Good to know @taalexander! If you end up filing an issue over over on aer after this is implemented (and deem it a good first issue), I’d be happy to take a look. I just got started with these repos yesterday.

[taalexander]

Sure, created an issue here https://github.com/Qiskit/qiskit-aer/issues/891.