OpenQuantum support#1169
Open
kuhnboy wants to merge 2 commits intoqBraid:mainfrom
Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
qbraid/runtime/openquantum/provider.py (1)
205-212: Don't swallow enrichment failures silently.If the detail fetch starts failing, this quietly drops
constraint_dataand leaves partially populated profiles with no breadcrumb. Catch the specific client error and log the backend id instead ofexcept Exception: pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qbraid/runtime/openquantum/provider.py` around lines 205 - 212, Replace the broad except/pass around the get_backend_class_details call with handling for the specific client error thrown by the session (e.g., catch the session/client library's specific exception class instead of Exception) and log a warning that includes the backend id (device_id) and the caught exception; keep falling back to the basic device data but ensure the log message references self.session.get_backend_class_details(device_id) and that you still do not re-raise for non-fatal enrichment failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@qbraid/runtime/openquantum/device.py`:
- Around line 146-163: The selection logic in device.py (symbols:
execution_plan_id, queue_priority_id, quote, plan["queue_priorities"],
plan["price"], q["price_increase"]) currently picks the cheapest plan first
which can pick a plan that doesn't offer the requested priority or not minimize
total cost; change it to select a valid plan+priority pair: if execution_plan_id
is provided, find that plan and then validate/choose the priority in
plan["queue_priorities"]; if only queue_priority_id is provided, first filter
quote for plans that contain that queue_priority_id and select among them; if
neither is provided, choose the plan and queue_priority pair that minimizes
total_cost = plan.get("price", 0) + q.get("price_increase", 0); raise ValueError
if a requested execution_plan_id or queue_priority_id cannot be found in the
filtered set.
In `@qbraid/runtime/openquantum/job.py`:
- Around line 79-88: The result() method in OpenQuantumJob accesses
self.device.id which raises when the job was loaded standalone without a device;
update result() (in class OpenQuantumJob / method result) to safely handle
missing device by checking for the attribute or using getattr(self, "device",
None) and only reading .id when device is not None, otherwise set device_id to
an empty string; ensure no other direct uses of self.device occur in result() so
standalone loaded jobs return properly.
- Around line 90-114: The code currently forwards the entire job_data via
enhanced_job_data into Result(...), which can leak transport fields like
output_data_url into Result.details; instead build a whitelist of allowed public
fields (e.g., explicitly pick/assign execution_plan, queue_priority and any
other safe metadata you intend to expose) and pass only those explicit keys to
Result (or set them as explicit keyword args) rather than **enhanced_job_data;
update the area around enhanced_job_data,
execution_plan_reverse/queue_priority_reverse and the Result(...) call to
construct and use a filtered_payload (or explicit kwargs) so sensitive transport
fields are never included.
---
Nitpick comments:
In `@qbraid/runtime/openquantum/provider.py`:
- Around line 205-212: Replace the broad except/pass around the
get_backend_class_details call with handling for the specific client error
thrown by the session (e.g., catch the session/client library's specific
exception class instead of Exception) and log a warning that includes the
backend id (device_id) and the caught exception; keep falling back to the basic
device data but ensure the log message references
self.session.get_backend_class_details(device_id) and that you still do not
re-raise for non-fatal enrichment failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3528f8ec-f40a-4a99-a78d-b921f9b7653b
📒 Files selected for processing (8)
CHANGELOG.mdqbraid/runtime/__init__.pyqbraid/runtime/loader.pyqbraid/runtime/openquantum/__init__.pyqbraid/runtime/openquantum/device.pyqbraid/runtime/openquantum/job.pyqbraid/runtime/openquantum/provider.pyqbraid/runtime/result.py
Comment on lines
+146
to
+163
| # Plan & priority selection (your logic, slightly cleaner) | ||
| if execution_plan_id: | ||
| plan = next((p for p in quote if p["execution_plan_id"] == execution_plan_id), None) | ||
| if not plan: | ||
| raise ValueError(f"Execution plan '{execution_plan_id}' not found in quote.") | ||
| else: | ||
| plan = min(quote, key=lambda p: p.get("price", float("inf"))) | ||
|
|
||
| if queue_priority_id: | ||
| prio = next( | ||
| (q for q in plan["queue_priorities"] if q["queue_priority_id"] == queue_priority_id), | ||
| None, | ||
| ) | ||
| if not prio: | ||
| raise ValueError(f"Queue priority '{queue_priority_id}' not found in selected plan.") | ||
| else: | ||
| prio = min(plan["queue_priorities"], key=lambda q: q.get("price_increase", float("inf"))) | ||
|
|
There was a problem hiding this comment.
Select the cheapest valid plan/priority combination, not the cheapest plan first.
This fallback can overcharge users and can also reject a valid request. If queue_priority_id is set without execution_plan_id, Line 152 may choose a plan that doesn't offer that priority even though another quoted plan does. Even with no inputs, minimizing plan.price before price_increase is not the same as minimizing total cost.
Proposed fix
- if execution_plan_id:
- plan = next((p for p in quote if p["execution_plan_id"] == execution_plan_id), None)
- if not plan:
- raise ValueError(f"Execution plan '{execution_plan_id}' not found in quote.")
- else:
- plan = min(quote, key=lambda p: p.get("price", float("inf")))
-
- if queue_priority_id:
- prio = next(
- (q for q in plan["queue_priorities"] if q["queue_priority_id"] == queue_priority_id),
- None,
- )
- if not prio:
- raise ValueError(f"Queue priority '{queue_priority_id}' not found in selected plan.")
- else:
- prio = min(plan["queue_priorities"], key=lambda q: q.get("price_increase", float("inf")))
+ candidates = []
+ for candidate_plan in quote:
+ if execution_plan_id and candidate_plan["execution_plan_id"] != execution_plan_id:
+ continue
+ for candidate_prio in candidate_plan.get("queue_priorities", []):
+ if queue_priority_id and candidate_prio["queue_priority_id"] != queue_priority_id:
+ continue
+ total_price = candidate_plan.get("price", float("inf")) + candidate_prio.get(
+ "price_increase", float("inf")
+ )
+ candidates.append((total_price, candidate_plan, candidate_prio))
+
+ if not candidates:
+ raise ValueError(
+ "No matching execution plan / queue priority combination was returned in the quote."
+ )
+
+ _, plan, prio = min(candidates, key=lambda item: item[0])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Plan & priority selection (your logic, slightly cleaner) | |
| if execution_plan_id: | |
| plan = next((p for p in quote if p["execution_plan_id"] == execution_plan_id), None) | |
| if not plan: | |
| raise ValueError(f"Execution plan '{execution_plan_id}' not found in quote.") | |
| else: | |
| plan = min(quote, key=lambda p: p.get("price", float("inf"))) | |
| if queue_priority_id: | |
| prio = next( | |
| (q for q in plan["queue_priorities"] if q["queue_priority_id"] == queue_priority_id), | |
| None, | |
| ) | |
| if not prio: | |
| raise ValueError(f"Queue priority '{queue_priority_id}' not found in selected plan.") | |
| else: | |
| prio = min(plan["queue_priorities"], key=lambda q: q.get("price_increase", float("inf"))) | |
| # Plan & priority selection (your logic, slightly cleaner) | |
| candidates = [] | |
| for candidate_plan in quote: | |
| if execution_plan_id and candidate_plan["execution_plan_id"] != execution_plan_id: | |
| continue | |
| for candidate_prio in candidate_plan.get("queue_priorities", []): | |
| if queue_priority_id and candidate_prio["queue_priority_id"] != queue_priority_id: | |
| continue | |
| total_price = candidate_plan.get("price", float("inf")) + candidate_prio.get( | |
| "price_increase", float("inf") | |
| ) | |
| candidates.append((total_price, candidate_plan, candidate_prio)) | |
| if not candidates: | |
| raise ValueError( | |
| "No matching execution plan / queue priority combination was returned in the quote." | |
| ) | |
| _, plan, prio = min(candidates, key=lambda item: item[0]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@qbraid/runtime/openquantum/device.py` around lines 146 - 163, The selection
logic in device.py (symbols: execution_plan_id, queue_priority_id, quote,
plan["queue_priorities"], plan["price"], q["price_increase"]) currently picks
the cheapest plan first which can pick a plan that doesn't offer the requested
priority or not minimize total cost; change it to select a valid plan+priority
pair: if execution_plan_id is provided, find that plan and then validate/choose
the priority in plan["queue_priorities"]; if only queue_priority_id is provided,
first filter quote for plans that contain that queue_priority_id and select
among them; if neither is provided, choose the plan and queue_priority pair that
minimizes total_cost = plan.get("price", 0) + q.get("price_increase", 0); raise
ValueError if a requested execution_plan_id or queue_priority_id cannot be found
in the filtered set.
Comment on lines
+79
to
+88
| def result(self) -> Result: | ||
| """Return the result of the OpenQuantum job.""" | ||
| self.wait_for_final_state() | ||
| job_data = self.session.get_job(self.id) | ||
| if job_data.get("status", "") != "Completed": | ||
| raise QbraidRuntimeError(f"Job failed: {job_data.get('message')}") | ||
|
|
||
| results = self.session.download_job_output(self.id) | ||
| data = GateModelResultData(measurement_counts=results) | ||
| device_id = self.device.id if self.device else "" |
There was a problem hiding this comment.
Avoid self.device here when the job was loaded standalone.
QuantumJob.device raises when no device was attached. load_job(..., provider="openquantum") can create an OpenQuantumJob with only a job id/session, so Line 88 will fail before returning the downloaded result.
Proposed fix
- device_id = self.device.id if self.device else ""
+ device_id = self._device.id if self._device is not None else ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@qbraid/runtime/openquantum/job.py` around lines 79 - 88, The result() method
in OpenQuantumJob accesses self.device.id which raises when the job was loaded
standalone without a device; update result() (in class OpenQuantumJob / method
result) to safely handle missing device by checking for the attribute or using
getattr(self, "device", None) and only reading .id when device is not None,
otherwise set device_id to an empty string; ensure no other direct uses of
self.device occur in result() so standalone loaded jobs return properly.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Added
OpenQuantumProvider,OpenQuantumDevice, andOpenQuantumJobclasses implementing the qBraid runtime interface for Open QuantumSummary by CodeRabbit