-
Notifications
You must be signed in to change notification settings - Fork 19
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
Populate backend.configuration_dict
from target_info
endpoint
#666
Conversation
…t-superstaq into populate_backend_config
@@ -462,17 +462,6 @@ def process_dfe(self, ids: List[str]) -> float: | |||
""" | |||
return self._client.process_dfe(ids) | |||
|
|||
def target_info(self, target: str) -> Dict[str, Any]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is deleted cause it's not how qiskit users are supposed to be getting target info. They should be using backend.configuration
|
||
def test_default_options() -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary test: Don't affect coverage with it's removal and doesn't look like a regression test.
Notebooks check won't pass until a new version of server is released. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart from some very minor docstring nits (and #666 (comment) if it's relevant)
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
Co-authored-by: Bharath Thotakura <113555655+bharat-thotakura@users.noreply.github.com>
@bharat-thotakura I did need to apply your suggestion for the other keys though: #678. Just pushed a commit to handle undefined keys. Please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the changes locally and it seems fine to me. Though, I did notice that qss.SuperstaqProvider().get_backend(<target>).configuration_dict
yields a couple of duplicate information. Namely, 'backend_name'
<-> 'target'
and 'n_qubits'
<-> 'num_qubits'
list the same thing. I think this is because of self.configuration_dict.update(target_info)
, so it may be worth dropping the duplicate key info.
Good catch. I've removed them from the final dict. |
@vtomole No other changes from me 👍 |
Part of #342