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 HamiltonianModel class for configuring PulseSimulator #493

Merged

Conversation

chriseclectic
Copy link
Member

Summary

Adds a HamiltonianModel class to store the hamiltonian configuration parts of the OPSystem class and hamiltonian utility functions from pulse qobj digest.

Details and comments

Currently I just refactored digest to use this class without actually modifying OPSystem (it builds the HamiltonianModel, then copies entries back to OPSystem) so as to minimize code changes. In a future PR this could be refactored so OPSystem stores the HamiltonianModel.

The pulse utility functions should also be refactored (or removed) to use this class.

@chriseclectic chriseclectic added the PulseSimulator Issues related to (future) pulse simulator label Dec 11, 2019
@chriseclectic chriseclectic added this to the Pulse Simulator milestone Dec 11, 2019
Copy link
Contributor

@DanPuzzuoli DanPuzzuoli left a comment

Choose a reason for hiding this comment

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

Looks good to me; it's a good start to refactoring digest.

qiskit/providers/aer/openpulse/hamiltonian_model.py Outdated Show resolved Hide resolved
ground_state += estate_coef * op.basis(len(self._evals), idx)
return ground_state

def calculate_frequencies(self, qubit_lo_freq=None, u_channel_lo=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the name of this is good for this iteration. Something that might make sense next, as we refactor so that OPSystem holds a HamiltonianModel, is to break this function up: one (or several) in HamiltonianSystem that compute eigenvectors/eigenvalues/dressed energies for the drift Hamiltonian, and then another in OPSystem called something like channel_frequencies, which, if passed the argument from_hamiltonian will draw the relevant info from HamiltonianModel.

Copy link
Member

@atilag atilag left a comment

Choose a reason for hiding this comment

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

LGTM!
Just some comments on styling

qiskit/providers/aer/openpulse/hamiltonian_model.py Outdated Show resolved Hide resolved
qiskit/providers/aer/openpulse/hamiltonian_model.py Outdated Show resolved Hide resolved
qiskit/providers/aer/openpulse/hamiltonian_model.py Outdated Show resolved Hide resolved
@chriseclectic chriseclectic merged commit 8fdfff2 into Qiskit:openpulse-sim Dec 13, 2019
chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Add HamiltonianModel class for pulse simulator

* update digest to use HamiltonianModel

* change statevector pulse tests to use state_fidelity
chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Add HamiltonianModel class for pulse simulator

* update digest to use HamiltonianModel

* change statevector pulse tests to use state_fidelity
chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Add HamiltonianModel class for pulse simulator

* update digest to use HamiltonianModel

* change statevector pulse tests to use state_fidelity
chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Add HamiltonianModel class for pulse simulator

* update digest to use HamiltonianModel

* change statevector pulse tests to use state_fidelity
@chriseclectic chriseclectic deleted the pulse-hamiltonian-model branch April 20, 2020 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PulseSimulator Issues related to (future) pulse simulator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants