Repository navigation
Feature/polish calibration - #37
Merged
Merged
Conversation
…ifting The kind column never gained quantify_tuner or qblox_tuner, so registering one passed every check the endpoint makes and failed on the insert. Nothing had ever submitted the form, only read the dropdown. The migration would not have fixed an existing install either: it adds missing fields and never revisits one already there, so a select kept the values it was created with. It now widens every select on start, add-only. The same shape elsewhere: a calibration report's mode and status went from the driver's payload into two selects with only an emptiness check, and a rejected insert in the listener loses the report. Both are checked against the column, as is the dispatch endpoint's mode.
Every format declares its own contents for its service unit, and nFPM replaces the shared list rather than adding to it, so the config file qpi.service is started with never made it into any package. Aliased into each override. Also renames the deprecated archives.format_overrides.format, which was making goreleaser check fail on an otherwise valid config.
Six of the eight FIXMEs asked what the code does, and three answers are that it does not: - Dispatch is not blocked by the one-at-a-time rule. The endpoint queues; FetchNextCalibration is what withholds. - A calibration does not reach a running QPU driver, and does not hold back its jobs. Both executors load the device config in their constructor, and FetchNextJob knows nothing about calibration. Named in §11. - The dispatch endpoint is not rate-limited. --event-rate-limit bounds a driver's inbound NNG events; no HTTP route is limited. The other two were prose: a "number"/"double" pun nobody else could read, and a clause about a stub only visible mid-implementation. §6.9 asked for scipy and lmfit on the scheduler extras. scipy is a core dependency and nothing imports lmfit, so the extras carry neither.
The one-at-a-time rule was justified by a report nobody could attribute, which the code disproves two functions away: dispatchCalibration sends the request ID and handleCalibrationResult closes out that exact row. The real reason is only that one tuner is one worker on one chip, so a second dispatch would queue inside the driver rather than in the collection. Same wrong reason removed from FetchNextCalibration and its test.
lmfit reaches all four extras through quantify-core, which both schedulers depend on. Dropping the declaration was a correctness fix, not a slimming one.
Nothing stops two tuners being registered against one QPU, and each gets its own dispatcher, so a queue that waited per driver let both DAGs sweep the same qubits and both write the device YAML — whole per write, a mix of two runs afterwards. FetchNextCalibration now withholds while any request is running on that driver's QPU, read by traversing driver.qpu so no field is denormalised. It does not cover a tuner's own drift check, which schedules on an internal queue guarded by a per-process threading.Event. Recorded in RFC 0004 §11.
A QPU driver read quantify.device.yml once, in its constructor, so a calibration on the same node changed nothing until someone restarted it. It now re-reads the file between jobs when its (mtime, size, inode) has moved — inode because the write-back replaces by rename, which a same-second same-size check misses. Applied onto the live device rather than rebuilding one, so the compiler, the instrument coordinator and qblox's HardwareAgent keep pointing at it, no qcodes name is taken twice, and the cluster connection is untouched. Only the coupler bias is re-applied, being a current in a rack rather than a reference. A config that will not parse leaves the device as it was. This also covers a calibration done by hand: drop the file in place and the running driver picks it up.
Two QPU drivers would hand the same hardware two schedules; two tuners would sweep the same qubits and each write the device YAML. Serializing the calibration queue per QPU stops the dispatched case but not a tuner's own drift check, which never asks the server — so the second driver is refused at /api/op/drivers/connect. Scoped by operation rather than kind: a quantify_tuner and a qblox_tuner calibrate the same chip. custom is exempt, its operation being whatever its author wrote. Registration is untouched — a standby record harms nothing until it connects. A driver counts as connected only when its socket is attached and this server is dispatching to it, so neither a crash leaving status=online nor a server restart can wedge a QPU no driver is on.
A driver holds two NNG ports, two goroutines and a listener socket from connect onwards. Only handleDriverToggle released them, so: - a disconnect left them running (status went offline, nothing else happened) - a deleted record left them running against an id with no record - a failed Listen left the driver in activeDrivers with nothing serving it, and connect had already answered 200 All four release now. No grace period, because there is nothing to protect: the port pair is stored on the driver's record and stays reserved there, so a reconnect rebinds the same two. A listening socket does not enter TIME_WAIT either. Every driver is also marked offline at startup. `online` is an observation made by the process that held the socket; a row that survives a crash describes a server that no longer exists, and left alone it blocks that QPU's next driver.
calibrate_worker loaded it once before its loop, so changing which routines run meant restarting the tuner. A file that no longer parses fails that calibration instead of carrying on with the previous config — the opposite of the executor's device config, which does carry on. The device config holds parameters a job can still run against; this one decides which routines run, and quietly spending hours on the old selection is worse than saying so. The worker stays alive either way.
…walk A tuner held the device it read at startup, so a chip moved by anything else — a hand edit, a restore of yesterday's parameters — was calibrated from stale values and then overwritten with them by the write-back. Refreshed at DAG start in all three entry points, never between routines: a device moving mid-walk leaves a fit and the parameters it was measured against disagreeing, which is worse than a stale parameter. load_into_device is the inverse of serialise_device and generic over both schedulers for the same reason, reusing _UNCALIBRATED to skip structural keys. _device_config_path is a property whose setter arms the watcher, so a tuner cannot arm the path and forget the reload.
…re config The round-trip test is the one that matters: one executor, built over an uncalibrated config, plays an X gate that cannot land (0/400 excited), the file underneath it is replaced with what a real calibration wrote, and the same executor lands it (>0.9) — no reconstruction anywhere, on both schedulers. Every other test in that file builds a fresh executor over the calibrated file, which is the restart this removes. The tuner is not alive alongside it. Under quantify a device element is a qcodes instrument with a global name, so one process cannot hold two devices called q0 — which is why §8 runs them as separate processes sharing the file. It doubles as the hand-calibration case. The hardware config deliberately does NOT reload. It builds the instrument coordinator and the Cluster behind it, so applying a new one means closing a live connection to the rack and dialling it again, and a reconnect that fails leaves the driver with no coordinator and no way back — the device config falls back to what is in memory, this has nothing. Warned once per change instead.
RFC 0001 owns the connection lifecycle and said nothing about either change: the one-per-role 409 at connect, and the three states behind it — enabled as admin intent, status as observed liveness, and the in-memory lease whose release paths and startup reset stop a QPU being wedged by a driver nothing is connected to. operations.md gains the operator-facing version: what the 409 means, and what to do when it appears with nothing actually connected. The tuner README still said the device file was "read at start", and its write-back section did not mention that the process driver beside it now picks the file up on its next job.
Nothing in that path consulted the QPU. FetchNextJob filtered time_slots and quantum_jobs only, there is no StopQPUDistribution despite the comment claiming one, and submission is plain CRUD. So switching a QPU off did not stop jobs reaching it, `maintenance` was a select value with a dashboard colour and no reader or writer anywhere, and jobs kept running on a chip mid-calibration. One gate closes all three. QPUUnavailable returns the reason rather than a bool: an operator staring at an idle queue should not have to guess which it is. Gated at submission and at dispatch, which are not alternatives. Submission so a user is not left queueing into a QPU that is off; dispatch as well because submission-time state says nothing about the state minutes later. A job already accepted waits rather than being failed. A calibration is still dispatchable under maintenance — that is the state a chip is in while someone works on it — but not to a QPU switched off. calibration_requests gains `qpu`, set at create and backfilled for older rows: a blank one would be invisible to the per-QPU checks. The QPU card drew its pill from `status == "online"`, so maintenance rendered red, and the TS QPU type was missing `maintenance` while the column accepted it.
Maintenance had consequences but no way to reach it except a hand-rolled superuser PATCH. POST /api/op/qpu/maintenance rather than raw CRUD, because clearing it cannot just write `online` — that would claim a connection the QPU may not have. It derives the status the same way re-enabling does, now shared as db.ConnectedQpuStatus so the two cannot drift. GET /api/op/qpus/availability serves the reason a QPU is not taking jobs. Built in the dashboard instead, the three states would have a second author in TypeScript, which is how `maintenance` came to exist in the schema and nowhere else.
A QPUState event carries online | maintenance | disabled to every driver of that QPU. This closes the one path no server-side gate can reach: a tuner's drift check runs on its own clock and never asks the server, so two drift-monitoring tuners on one chip used to collide no matter what the queue did. Level-triggered from the dispatcher loop, not pushed from the endpoint that changed the state: the PUSH socket lives inside that goroutine, and comparing against the last state sent means nobody has to remember what was delivered. A reconnecting driver or a restarted server is simply told again — which is why restarting cannot bring a switched-off QPU back into service. The state, not a sleep/wake pair, because the right response differs by operation: a monitor keeps reporting a fridge under maintenance (asserted), a tuner stops its own checks but still honours a dispatched calibration, and neither runs on a QPU switched off. A driver told nothing assumes online. QPUServiceState is narrower than QPUUnavailable on purpose: a running calibration stops jobs but is not a service state, or the tuner doing the calibrating would be told to stop.
RFC 0005 itself says the calibration graph is wired for real hardware but "not run against it" — the changelog's "now runs on real hardware" claimed more than that.
A tuner's snippet named no -o options, so pasting it started a driver that looked for calibration.yml, the device config and the hardware config in whatever directory it happened to run in — under systemd, one the operator never chose. The same was true of the quantify and qblox QPU drivers. Only the InSnippet flag was missing. Set per kind, because processOptions() is one list shared by five kinds: qpu.py passes the quantify configs to whichever executor is built, so they belong in every process kind's schema, but a mock driver told where the quantify device config lives is a puzzle. No new env var is needed for systemd — DRIVER_OPTIONS already carries them, and install-systemd.sh already reads it. One -o per continuation line, emitted before each option so the command cannot end on a dangling backslash, which a test now pins for every kind. TestSnippetsExecutor asserted "a process driver takes no -o options" — true of the snippet, not of the driver.
The RFC 0004/0005 work session turned each feature into a multi-paragraph essay with physics rationale, measured numbers and revert/re-add backstory. That detail lives in the RFCs and the code; the changelog now matches the tagged one-line-bullet style used everywhere else in this file.
Audited every doc against the code it describes. The recurring cause is RFC
0004's third operation: `calibrate` landed, and the places that enumerate
operations were not all updated.
- root README: `/api/op/qpus/connect` is `/api/op/drivers/connect`, and
`drivers/create` and `drivers/toggle` were missing; drop the documented
`PATCH /api/admin/users/{id}`, which no longer exists — QPU seconds are
credited by the `qpu_time_requests` approval hook.
- py/go/js driver READMEs: operations are three, not "exactly these two"; the
Python CLI reference and its `--operation` help text omitted `calibrate`
entirely, and the capability summary omitted the two tuners.
- py client README and provider docstring: `QPIBackend(client, num_qubits=5)`
is not a valid call — the qubit count comes from the server. Use
`client.get_backend(name)`, as the package's own docstring already did.
- RFC 0001: the `EnableDriverFramework` rollout flag has been removed.
- RFC 0004: §7 has no hardware tier (§9's manual verification is what is
outstanding); `TestOperationsAreAClosedPair` is now `...ClosedSet`; drop a
line number that had drifted and a "confirm before this leaves Draft" note
on a citation now verified against the abstract.
- dashboard README was still the untouched Vite scaffold. It is the one doc
no check covered, so add it and the tuners README to check_docs.py — both
were reachable only through a symlink the walker skips.
The route table was corrected against the code on a branch that predated `qpu/maintenance` and `qpus/availability`, so rebasing left it accurate but short by two rows.
RFC 0004's change table cited `TestOperationsAreAClosedPair`, which is what the test was called before `calibrate` made "pair" wrong. The note's history is right; the identifier was no longer greppable. Checked the rest of that table while there — every other symbol it names still exists. The root README's component list was the one other place still describing drivers as process and monitor, "both".
Every /api/op route is superuser-only, except handleDriverConnect which
authenticates by driver token. The availability endpoint I added had no check at
all, so it was the one public route in an admin namespace.
It is discovery, not an operation: a client deciding where to send a job needs it,
and it says nothing /api/qpus does not already say about a QPU's status. So it
moves to /api/qpus/availability for the list and /api/qpus/{name}/availability for
one. Both public, both delegating to scheduler.QPUUnavailable.
The list exists because asking per QPU is one request per row, which does not
scale past a handful. Still derived on read rather than stored on the record: the
answer is a function of enabled, status and whether a calibration is running, so a
copy on the record would need updating from four places and would be wrong the
first time one was missed.
Resolved by name, which is what the route says. The sibling GET /api/qpus/{name}
passes its path value to FindOne, which resolves a record id — so it answers only
for callers who pass an id. Left alone here, but worth fixing.
driver_test.go had no driver.go. The register-and-connect flow it covers is handleDriverCreate and handleDriverConnect, so it joins api_test.go; joinKinds is in utils.go, so its test starts utils_test.go. Every _test.go in the tree now names a source file beside it.
scqubits and qutip were a dependency group, which never reaches the built wheel — so a user could not install what -o is_simulated=true needs. The tuner README worked around it by telling people to pip install both by hand, unpinned. They are a `sim` extra now, and provides-extras in the lock confirms it ships. Omitting it used to surface as ModuleNotFoundError from inside eigenvalues(), hours into a calibration and naming a package rather than the extra. The simulated coordinator now checks at construction, which is the one place all four entry points pass through.
QPUUnavailable returned a string, so its name read as a boolean and three lines of comment had to say otherwise. It is UnavailableReason now, and QPUServiceState is ServiceStateOf. The state strings they return are constants rather than repeated literals, so the possible values no longer need documenting in prose. ConfigFile.accept() said nothing about what it does; it is mark_read(). Its body was `if a or b: return False; return True`, now the expression it always was. The tuner and the executors had two names for one operation — load_into_device and apply_device_config. Both are apply_device_config, which is also the inverse of save_device_config beside it. lastState -> lastStateSent, which is the distinction the comment above it existed to draw. Docstrings and comments cut throughout to what the code cannot say.
The sections I added to RFC 0001 §4/§5, RFC 0004 §6.8/§8/§10, operations.md and the tuner README said the same things at half again the length they needed. Every claim survives; the restatements and the throat-clearing do not. 31 lines lighter across the four.
7% shorter. Every claim kept except one: that the kind→extra mapping "formalises what install-systemd.sh does today", which was rationale for a design that now exists in the catalog. Three things were wrong rather than merely long: - §1 and §5 spoke in the present tense about a state of affairs this RFC replaced, while the header says Implemented. - §7 described the drivers collection as living "behind the flag". §5 says the flag was removed. - §7 and §12 pointed at §11 for retention. §11 is the implementation plan; the retention setting is in §12. §11 also promised a phased plan "maintained separately from this RFC", which is not in the repository. It now says the work is complete and where the plan went.
18% shorter, every claim kept. It said most things four or five times over. The worst was §3.4, which repeated all twenty default colour values already given as Go in §3.1 — it now shows the shape and points at them. §3.6 restated §3.2's cache lifecycle; §6.1 restated §3.5's access rules; §5.1 walked through the sequence diagram immediately above it. Each is now a pointer. §7's "After" was ten bullets recapping §3-§6, of which one carried a fact of its own: the dashboard works with zero themes configured. §8 explained what each mechanism does before saying why it was chosen; only the why was not already above. Vocabulary entries had grown into paragraphs that argued rather than defined.
Only 5% shorter — this one was written tightly. But §1 still described the design §9 says was reversed: that a DeviceSpec declares what -o options a device takes, and that --help can therefore list every option. §5, §9 and §13.6 all say there is no option schema, and the shipped --help says the keys are QPI-UI's to document. §1 now matches. §12 promised a plan "maintained separately", which is not in the repository — same dangling pointer RFC 0001 §11 had. §13's nine decisions each restated the rationale their own section gives; they are statements plus pointers now. The Rejected paragraph is untouched: it is the only record of what was considered and turned down. The reason for removing --name is kept in §11, where someone migrating will look, and dropped from §4.
Three of the four were mine, from edits earlier today: - §4's decisions table still said the fitting dependencies go into the quantify and qblox extras. §6.9 says the tuners add no dependency, because scipy is core and nothing imports lmfit. I changed §6.9 and not the table summarising it. - Two places said test-py-sim syncs `--group sim`. It is an extra now, since a dependency group never reaches the built wheel. - §12 promised a plan "maintained separately", as RFC 0001 §11 and RFC 0003 §12 did. §11 listed fourteen faults found during implementation, mostly narrating how each was diagnosed and fixed — history that belongs in the change log. It now carries only what outlives the fixes: nine facts about the chips, the schedulers and the sweeps that the next routine author needs. The two schedulers' differences in parameter naming and in shape are kept and consolidated; so are the chevron contrast guard, the punchout coupling, the 3% spectroscopy drive and the readout LO arrangement. The methodological notes — that end-to-end testing found these, that two were encoded as expectations, that only exercising one scheduler was the common cause — are kept. 6% shorter overall, with §11 halved.
Barely shorter — 1% — because almost none of this document is padding. What it had instead were defects from being updated in place as the work progressed: - Phase 6 appeared twice: once with its sub-items, once as a struck-through "Done" stub with nothing under it. - "§12b" was a section wedged in after §12. It is a subsection of §12 now, which is what it always was. - Ten struck-through headings, each followed by "Done" or "Decided" saying the same thing, and fifteen item-level "**done.**" markers. §12 is no longer a plan being ticked off, so it is retitled to what it is: how the graph was built. I went looking for duplication between §12's node-by-node record and §14's summary of divergences, expecting to cut. Only one claim was genuinely stated twice — the integration_time reasoning — and §14 now points at §12 for it. The rest reads like overlap and is not: §14 says frequency and amplitude are one node because the resonance moves with power, while §12 gives the deeper reason, that optimising for discrimination moves away from the magnitude contrast every other routine reads.
Every component that declares a version: - qpi-ui (main.go, the dashboard's package.json, the WiX installer manifest) - qpi-driver py, js and the Go CLI's Version var - qpi-client py and js The Python packages read their version from installed metadata and fall back to a literal, so the literal moves too — and with it the two tests that assert the fallback path by patching metadata to raise. Five lockfiles regenerated. The README's release URLs move as well; they 404 until the tag exists. Makefile's VERSION is bumped for consistency but referenced nowhere else in it.
Making sim an extra changed how it is installed, and I only updated the Makefile and the CI workflow. e2e/lib.sh still passed `--group sim`, so any run with QPI_E2E_SIMULATED=1 died at the sync with "Group `sim` is not defined". Its pip fallback on the next line already said `[...,sim]`, so that branch had been wrong the other way round before today and is now right too. Fourteen further places still called it a dependency group — three test modules' importorskip reasons, the pytest marker description, three docstrings, the cypress script's header comment and a changelog line. All say extra now.
Renaming "Driver Enable Control" to "Service", and "Online (Enabled)"/"Offline (Disabled)" to "In service"/"Switched off", broke three specs that asserted the old text. The label was wrong on a QPU card — it described `enabled`, admin intent, in the words of `status`, which is observed — but I changed it without grepping the specs. user-view.cy.ts was asserting the absence of strings that no longer appear anywhere, so it had quietly stopped testing anything. It now names the current labels, and checks the maintenance toggle is admin-only too, which nothing did. DriverCard keeps "Driver Enable Control": there it is accurate.
ch-ahindura
force-pushed
the
feature/polish-calibration
branch
from
August 4, 2026 17:39
899a59c to
4d496da
Compare
Tinitto
self-requested a review
August 4, 2026 17:39
This branch was previously deployed
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
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.
Why
We needed to fix some bugs and clean up a few documentation mishaps
Added
qpi-driver: asimextra, so-o is_simulated=trueno longer needsscqubitsand
qutipinstalled by hand:pip install 'qpi-driver[cli,quantify_tuner,sim]'.qpi-ui: admins can put a QPU under maintenance from the dashboard. It then takesno jobs, but can still be calibrated.
qpi-ui: a QPU that is not taking jobs says why, on its card.GET /api/qpus/availabilityand/api/qpus/{name}/availabilityserve the reason.QPUStateevent, in all three SDKs: a driver is told when its QPU goes online,under maintenance or disabled. A tuner stops its own drift checks unless online.
qpi-driver: config files are re-read when they change on disk, so a calibrationtakes effect without a restart. A new element and the hardware config still need one.
Fixed
qpi-ui: no tuner could be registered — thekindcolumn never gainedquantify_tunerorqblox_tuner.qpi-ui: the schema migration never revisited a field it had already created, so aselect kept its original values forever.
qpi-ui: a calibration report'smodeandstatusare checked before insert,rather than failing inside the listener and losing the report.
qpi-ui: switching a QPU off, maintenance, and a calibration in progress now stopjobs — refused at submission with the reason, held back at dispatch. Queued jobs
wait rather than fail.
qpi-ui: two tuners on one QPU could calibrate it at once. Calibration serialisesper QPU, and connect returns 409 for a second driver of the same operation.
qpi-ui: a driver's ports and goroutines are released when it disconnects, isdeleted, or fails to bind — previously only when disabled. Drivers are marked
offline at startup.
qpi-ui: a QPU under maintenance is no longer drawn as offline.qpi-ui: the setup snippets for a tuner, and for the quantify and qblox QPUdrivers, now pre-fill the config paths those drivers cannot start without.
qpi-ui: the deb, rpm and apk packages ship/etc/qpi.config.yml.qpi-driver: thequantifyandqbloxextras no longer declarescipy(alreadya core dependency) or
lmfit(imported nowhere).Changed
docs: Removed redundancies in documentation