Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Stop using basic aer internal private attributes #1554

Merged

Conversation

mtreinish
Copy link
Contributor

@mtreinish mtreinish commented Mar 16, 2021

Summary

This commit fixes the usage of internal only explicitly private
attributes of basicaer's job class. These look they were used to
manually create a job id which was manually set ahead of time despite
still being a random uuid and then launching a simulation from that
constructed job object. Which is both the incorrect order to run a
simulation (runs generate jobs, not the other way around) but also
served no functional purpose. This commit removes this as this is a
blocker for Qiskit/qiskit#5128 which is upgrading the basic aer
provider to use the latest versioned provider interface which removes
all of those private methods and also moves to a synchronous execution
model.

Details and comments

This needs to be included in the next 0.9 release to unblock Qiskit/qiskit#5128 (I had to make the same change in that PR to fix basic aer usage with the migrated code in terra)

This commit fixes the usage of internal only explicitly private
attributes of basicaer's job class. These look they were used to
manually create a job id which was manually set ahead of time despite
still being a random uuid and then launching a simulation from that
constructed job object. Which is both the incorrect order to run a
simulation (runs generate jobs, not the other way around) but also
served no functional purpose. This commit removes this as this is a
blocker for Qiskit/qiskit#5218 which is upgrading the basic aer
provider to use the latest versioned provider interface which removes
all of those private methods and also moves to a synchronous execution
model.
Fix copyright
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

From what I recall the code has been around quite some time. It was more or less a copy of the code from the simulators that created and submitted a job. It was done so we could skip doing the qobj validation, which the simulators used to do, which used to take a significant amount of time. At least this is my best recollection. If it can be simpler now its fine.

@mtreinish
Copy link
Contributor Author

From what I recall the code has been around quite some time. It was more or less a copy of the code from the simulators that created and submitted a job. It was done so we could skip doing the qobj validation, which the simulators used to do, which used to take a significant amount of time. At least this is my best recollection. If it can be simpler now its fine.

Yeah, the simulator run path used to actually do validation 3 times, once in qobj generation via marshmallow, once in jsonschema, and then once again in the simulator code, but even once was pointless and didn't accomplish anything (since it was all internal to qiskit). It was a real mess and a complete waste of electricity. I removed all of that a long time ago once I saw how awful it all was.

Most of this code (in the migrated terra side version) will need to be removed anyway since it doesn't serve a purpose with the new backends interface (and there not being a qobj anymore), but that's a later refactor.

@manoelmarques manoelmarques merged commit 7229dd1 into qiskit-community:master Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants