Skip to content

QDAC2 wiring#103

Open
TheoLaudatQM wants to merge 18 commits into
release/nightlyfrom
feature/qdac_wiring
Open

QDAC2 wiring#103
TheoLaudatQM wants to merge 18 commits into
release/nightlyfrom
feature/qdac_wiring

Conversation

@TheoLaudatQM

@TheoLaudatQM TheoLaudatQM commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Description

This branch completes end-to-end QDAC2 wiring and configuration for quantum-dot QUAM builds: structured QDAC entries in machine.wiring, automatic QdacSpec attachment from that wiring (no manual dac_mapping), logical (non-resolvable) QDAC reference strings to avoid QUAM #/ pointer warnings, and DAC driver configuration stored in wiring.json under dac_config (aligned with how network is stored), with optional injection via build_quam_wiring(..., dac_config=...).

High-level changes:

  • Wiring shape: qdac_output / qdac_trigger blocks with unit_index, port, and a stable ref string (logical path, not a QUAM JSON pointer—no #/ prefix) plus legacy fallbacks where applicable.
  • Builder: build_base_quam / build_quam apply QDAC specs using _dac_mapping_from_wiring, mirroring Stage 1 gate IDs and barrier indexing; dac_mapping was removed from the public API.
  • add_dacs: Takes only the machine (and optional kwargs); always derives the gate map from wiring.
  • Persistence: BaseQuamQD.dac_config is serialized with wiring and network in wiring.json; set_dac_config updates that field. Separate dac/*.jsonc write/read paths are removed (no legacy migration on load).
  • Ports: Ensures a ports container exists for QDAC-only connectivity so machine state remains valid.
  • Allow QDAC only components that won't appear in the opx configuration (machine.generate_config()).
  • Tests: Integration coverage for structured QDAC wiring, QdacSpec from wiring, dac_config round-trip in wiring.json, etc.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🔧 Refactoring (no functional changes)
  • ✅ Test addition/update
  • 🎨 Code style/formatting update

Related Issues

Related to PR-341 on qualang-tools

Checklist

Code Quality

  • My code follows the project's style guidelines
  • I have run pre-commit hooks and all checks pass (pre-commit run --all-files)
  • My commits follow the Conventional Commits format
  • I have performed a self-review of my code
  • My changes generate no new warnings

Testing

  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally (pytest)
  • I have tested my changes with the example scripts (if applicable)

Documentation

  • I have updated the documentation (if needed)
  • I have added docstrings to new functions/classes
  • I have updated the CHANGELOG.md (if applicable)
  • I have checked my code and corrected any misspellings

Breaking Changes

Does this PR introduce breaking changes? Yes

  • dac_mapping removed from build_base_quam and build_quam. QDAC output/trigger metadata must come from machine.wiring (qdac_output / legacy qdac_channel, etc.). Overrides previously done only via dac_mapping should be expressed in wiring or by post-build APIs (e.g. BaseQuamQD.wire_voltage_gate_qdac / apply_qdac_channel_mapping) if still needed.
  • add_dacs(machine, ...) no longer accepts a mapping argument; wiring is the single source of truth for which gates get QdacSpec.
  • DAC configuration persistence: dac_config now lives in wiring.json. The old dac/*.jsonc layout is no longer written or read on save/load. Existing repos that relied on that folder must migrate entries into wiring.jsondac_config (or set dac_config before save).
  • ref strings under qdac_output / qdac_trigger are no longer #/qdac/... QUAM pointers; they are logical paths (e.g. qdac/analog_outputs/qdac1/1). Anything that assumed a #/ prefix must be updated.

Migration guide (summary):

  1. Regenerate or edit wiring.json: add dac_config with the same structure previously passed to set_dac_config (per-DAC keys like qdac1, qdac2, …).
  2. Remove reliance on dac_mapping in build_base_quam / build_quam; ensure wirer/builder emits qdac_output (and triggers if used) on the relevant lines.
  3. Optionally pass dac_config into build_quam_wiring(...) when creating wiring so the first save already contains network + wiring + DAC config together.
  4. Drop dependence on dac/*.jsonc; merge those files into dac_config in wiring.json if you still have old state directories.

Testing Details

  • Environment: Python 3.11+ recommended (project/tests may depend on StrEnum and other 3.11 APIs); confirm in CI.
  • Automated: Run pytest for quam-builder (e.g. tests/builder/quantum_dots/test_wirer_builder_integration.py and related builder tests).
  • Manual / scripts: Run a flow that calls build_quam_wiring (with and without dac_config), then build_base_quam / build_quam, save, reload, and confirm:
    • No spurious Could not get reference "#/qdac/..." warnings from wiring traversal.
    • wiring.json contains wiring, network, and dac_config as expected.
    • connect_to_external_source works when dac_config is populated and QdacSpec.dac_name keys match dac_config entries.

Screenshots/Examples

Example dac_config (in wiring.json):

{
  "wiring": { },
  "network": { "host": "...", "port": null, "cluster_name": "..." },
  "dac_config": {
    "qdac1": {
      "driver_module": "qcodes_contrib_drivers.drivers.QDevil.QDAC2",
      "driver_class": "QDac2",
      "connection": { "visalib": "@py", "address": "TCPIP::<ip>::5025::SOCKET" },
      "channel_method": "channel",
      "accessor": "dc_constant_V",
      "is_qdac": true
    }
  }
}

@TheoLaudatQM TheoLaudatQM changed the base branch from main to release/nightly April 9, 2026 14:26
@TheoLaudatQM

Copy link
Copy Markdown
Collaborator Author

@sebastianorbell-qm regarding the commit where I added the default baseband pulses, I didn't update the function register_default_component_pulse_factories as I didn't fully understand how it works. Shall I somehow update it with the baseband pulse functionality?

@sebastianorbell-qm

Copy link
Copy Markdown
Contributor

@TheoLaudatQM Your confusion is valid, the register_default_component_pulse_factories() architecture is not wired up yet, I began to build it to align with the methods of adding default macros but I think I will remove it. The real default pulse materialization should happen currently in _ensure_default_pulses(), in the same macro_engine/wiring file.

I think default pulses require less flexibility than macros so maybe there is an argument for having them aligned more tightly with the quam construction - I have been intentional about creating separation with macro definitions

>>> gate.offset_parameter() # Returns 0.1V
"""

opx_output: Any = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense - but maybe should throw an error if both opx_output and dac_spec are None

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do you think it is necessary?
In the end you might want to be able to connect something else that a DAC or an OPX channel, but I am not sure.
Also, currently nothing will really happen if a user sets both of them to None.

module = importlib.import_module(config["driver_module"])
dac_class = getattr(module, config["driver_class"])
if dac_name in dac_instances:
dac_instances[dac_name]["driver"].close()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why there is a .close(), any chance you can explain

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You cannot open two QDAC2 instance with the same name, so if one instance already exists when the user is trying to connect to the QDAC2, then we will first close the existing connection and re-open a new one afterward. An alternative could be to just ignore the new connection if one instance already exists, but I thought that the user might have a good reason for re-opening it.

self.qmm = QuantumMachinesManager(**settings)

## TODO: need to also call self.create_virtual_dc_set("main_qpu") every time?
if self.dac_config and ~skip_dacs:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if skip_dacs is a bool, then ~skip_dacs will result in
skip_dacs=True then ~skip_dacs -> -2
and skip_dacs=False then ~skip_dacs -> -1
as ~ is a bitwise operator.

We should use not skip_dacs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I fixed it.
@KU-QM Regarding the TODO comment here, do you know if we need to call create_virtual_dc_set("main_qpu") every time we connect to the DAC to update the voltages, of if it is only required once at the beginning to set the virtual dc set?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should not have to do it every time. That function will create it and store as a Quam attr, and it will know to apply voltages via the offset_parameter.
You do need to ensure that the VoltageGate's offset_parameter is wired correctly to the DAC driver, which is what the connect_to_external_source function was designed to do

@TheoLaudatQM TheoLaudatQM Apr 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Cool thanks @KU-QM, this is also what I thought, but create_virtual_dc_set("main_qpu") is querying the actual DC voltages inside the add_layer() method, so I was surprised:

    def add_layer(
        self,
        source_gates: List[str],
        target_gates: List[str],
        matrix: List[List[float]],
        layer_id: str = None,
    ) -> VirtualizationLayer:
        """
        Adds a new virtualization layer to the VirtualDCSet.

        Args:
            source_gates: A list of names for the virtual gates in this layer.
            target_gates: A list of names for the physical (or underlying virtual)
                          gates that this layer maps to.
            matrix: The virtualization matrix defining the transformation.

        Returns:
            The created VirtualizationLayer object.
        """
        self._validate_new_layer(layer_id, source_gates, target_gates, matrix)

        matrix_array = np.array(matrix, dtype=float)
        use_pseudoinverse = matrix_array.shape[0] != matrix_array.shape[1]

        virtualization_layer = VirtualizationLayer(
            id=layer_id,
            source_gates=source_gates,
            target_gates=target_gates,
            matrix=matrix,
            use_pseudoinverse=use_pseudoinverse,
        )
        self.layers.append(virtualization_layer)

        # Populate the new layer
        **all_voltages = self.all_current_voltages
        self._current_levels.update(all_voltages)**

Are the last two line really needed here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You mean that we should be able to create the virtual DC set without connecting to the DAC?

The reason that those last two lines are there, is so that whenever a new layer is created, the cached dictionary of virtual voltages (_current_levels) is immediately kept up to date. We want to avoid someone using get_voltages() after adding a layer and running into an error that says the cached dict _current_levels does not have that key.

However, I do see your point that perhaps we would like to create the DC set without actually interfacing with the DAC. Perhaps the best strategy here is to have a bool in add_layer and add_to_layer with the option to query the DAC, defaulting to True. In the BaseQuamQD function, you could set that to False, so that we can create the set without querying the DAC. Thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh interesting. I think that it makes sense but if nothing weird happened, then why not using the cached values directly self._current_physical_voltages instead of querying them with self.current_physical_voltages?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That could be a good option, to use _populate_virtual_gate_voltages(self._current_physical_voltages). I think I wanted to ensure that there was a sync, just in-case you have some other QDAC controller at some point. Perhaps the bool option could just use either self.current_physical_voltages if requery = True and self._current_physical_voltages if else. I think it would be nicer to have the option

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yep, sounds good. I'll make the modification then. Thanks!

Comment thread quam_builder/architecture/quantum_dots/qpu/base_quam_qd.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants