Skip to content

Modify manual output control to make it compatible with OPX1000#318

Open
cjwu-qm wants to merge 5 commits intomainfrom
modify-manual_output_control
Open

Modify manual output control to make it compatible with OPX1000#318
cjwu-qm wants to merge 5 commits intomainfrom
modify-manual_output_control

Conversation

@cjwu-qm
Copy link
Copy Markdown
Collaborator

@cjwu-qm cjwu-qm commented Dec 1, 2025

Update the manual output control python file so that it is compatible with OPX1000.
Also updated the manual so that it includes the new compatibility.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 1, 2025

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit e5432f4.

♻️ This comment has been updated with latest results.

if close_previous:
self.qmm.close_all_quantum_machines()
self.qmm.close_all_qms()
# self.qmm.close_all_quantum_machines()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please remove code if not needed

self.analog_qm = self.qmm.open_qm(self.analog_config, False)
self._start_digital_qms()
self._start_analog()
print("If an element is turned on without an explicit amplitude, the amplitude defaults to 0.5 V.")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Better practice towards production grade code will be to use logging module. Logging can stay in the code and just be configured differently for development vs. production.

for port_int in analog_ports:
if isinstance(port_int, tuple):
if len(port_int) == 2:
con_number = (port_int[0] - 1) // 10 + 1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As this calculation repeats many times in the code, please use something like:
def compute_con_number(port_index, group_size=10):
return (port_index - 1) // group_size + 1

con_number = compute_con_number(port_int[0])


if isinstance(port_int, tuple):
port_str = str(port_int)
port1 = (port_int[0] - 1) % 10 + 1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same for this calculation. Please define a function instead of replicating code

"I": (con, port1),
"Q": (con, port2),
},
"intermediate_frequency": 0e6,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A bit of neat picking ;) , will be better to define consts to all frequencies e.g: INTERMEDIATE_FREQUENCY = 0e6
Same apply for other frequencies explicitly written in the code.

@cjwu-qm
Copy link
Copy Markdown
Collaborator Author

cjwu-qm commented Feb 27, 2026

Summary

  • Remove stale commented-out QM cleanup code in ManualOutputControl.__init__ to keep the shutdown path clear and maintainable.
  • Replace constructor print(...) with module logging (logger.info(...)) so runtime messaging can be managed by application logging configuration.
  • Extract repeated OPX/OPX+ port-index arithmetic into helper functions (compute_con_number, compute_port_in_controller) and reuse them across analog/digital config generation.
  • Introduce DEFAULT_INTERMEDIATE_FREQUENCY_HZ and replace hard-coded intermediate-frequency literals (0e6 / 0.0) with the named constant for consistency and readability.
  • Preserve existing behavior and config structure while cleaning up implementation details (including replacing identity comparison with value comparison in controller checks).

@cjwu-qm
Copy link
Copy Markdown
Collaborator Author

cjwu-qm commented Mar 2, 2026

@OziEgri I have modified according to your comments. Please let me know if you have further comments! Thank you.

@cjwu-qm cjwu-qm requested a review from OziEgri March 6, 2026 16:45
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.

2 participants