Skip to content
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

Review qubit operand separation in strings in old style configuration files #444

Open
jvansomeren opened this issue Apr 2, 2022 · 3 comments

Comments

@jvansomeren
Copy link
Collaborator

jvansomeren commented Apr 2, 2022

In old style configuration files in the instruction and decomposition sections keys (and decompositions) contain gates with operands. Those operands are identifiers such as q0, %0, etc. In those keys and decomposition sections, those operands are separated by a single comma optionally with spaces around them (that is the way that OpenQL accepts those rules).
But when those commas are omitted and only white space separates those operands, OpenQL cannot match those keys with actual gates with operands and/or when in decomposition rules operand separation no commas are used, the generated gates cannot be matched with instruction and decomposition rule keys. The end-effect is that particular gates are not decomposed and end up in the resulting qasm or at the entry of the code-generator. Examples have been seen of such behavior with cnot and swap gates (because these are the ones that occur often and need decomposition, also because swap decomposes to cnots).

Exactly which absence of commas is accepted and which result in non-matching has not been investigated yet.
What has been done, is a consistent replacement of sole white space by commas, which worked out well.

So the following are suspect in the decomposition section:

  • key: “swap q0 q1” (specialized decomposition)
  • key: “swap %0 %1” (parameterized decomposition); key: “swap” (parameterized decomposition) is ok
  • subinstruction in the decomposition: “cz q0 q1”
  • subinstruction in the decomposition: “cz %0 %1”

And in the instructions section:

  • key: “cz q0 q1” (specialized instruction)
  • key: “cz %0 %1” (parametrized instruction)

What needs to be done:

  • verify what is documented: for each of those cases above: a single comma, only spacing, or a comma with opt. spacing
  • check all json files in OpenQL (tests/.json and src/ql/arch//resources/*.json) on this
  • decide whether the practice of all json files in OpenQL is ok, i.e. an update of OpenQL should accept all current practice
  • decide on what will be ok and what not
  • update the documentation accordingly
  • verify what is silently accepted though not according to the documentation
  • solve that in OpenQL where json instructions/decompositions are read, and give the appropriate error when wrong
  • add tests
@jvansomeren
Copy link
Collaborator Author

Part of the problem is existing practice: some json files for cc and for diamond use a space as separator in the key in the gate_decomposition section and/or in the sub instructions.
Are the following used:

  • src/ql/arch/cc/resources/hwconf_default.json
  • tests/cc/cc_s5_direct_iq.json
  • src/ql/arch/diamond/resources/hwconf_default.json

Nowhere in the code is checked when reading a json file what is the case. Only some sanitation is done: removal of redundant spacing. Resulting keys are accepted and stored in the instruction_map.
When finding keys on gate creation in the instruction_map, a comma without spacing is required, hence those finds fail.

@wvlothuizen
Copy link
Collaborator

The documentation is quite clear that the operands should be a comma separated list:

The spaces as operand separator are probably a hangover of #214 (Jan 2019), and were already unsupported in https://github.com/DiCarloLab-Delft/OpenQL/blob/0.8.1.dev4/src/hardware_configuration.h#L157 (which is the version that now still runs on QI)

For reference, the extra check should be built into:

  • platform.cc, which handles the 'old' API, see Platform::load()
  • old_to_new.cc, see parse_instruction_name()

@jvansomeren
Copy link
Collaborator Author

jvansomeren commented Apr 4, 2022 via email

wvlothuizen added a commit to DiCarloLab-Delft/PycQED_py3 that referenced this issue Apr 4, 2022
…entries (containing " " instead of "," as separator) to subdir 'depr'. Also see QuTech-Delft/OpenQL#444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants