search

QuTech-Delft / OpenQL

OpenQL: A Portable Quantum Programming Framework for Quantum Accelerators. https://dl.acm.org/doi/10.1145/3474222
https://openql.readthedocs.io
Other
97 stars 44 forks source link

rOEE mapping algorithm implementation. #442

Open Hopery opened 2 years ago

Hopery commented 2 years ago

The previous pull request was #439, this new one is because of the change branch's name.

jvansomeren commented 2 years ago

Hi Anabel,

  1. As far as I can see, I'm still missing an option to enable/disable your algorithm. Can you add it?
  2. I saw your example.py file. Can you please turn it into a file in tests, e.g. tests/test_chong.py that checks whether the chong partitioning implementation is working? You then have to intercept the output (e.g. in tests/test_output/test_chong_last.qasm, when you call the test internally in the python file test_chong) and add it to the qasm files in the directory golden.
  3. You updated the multi-core-4x4 json file from the one in tests and used by the other tests. Is that wise? If so, please give it another name to avoid confusion. I would prefer that you use the one in tests with 16 cores since the other multi-core tests use that one as well.
  4. And after all that do a run of python3 -m pytest that demonstrates that the other tests pass.

Thanks!

Hopery commented 2 years ago

Hi Hans (@jvansomeren ),

I closed the last pull request to change the branch's name. In the previous pull request, you commented:

Hi Anabel, Would you please:

  1. merge in develop again (I merged in the updated channel resource just now)
  2. add an option to turn your code on/off
  3. turn the example .cc into a .py (you can use tests/test_multi_core.py as inspiration)

With that, I can compare your implementation against the current one, whether both pass all tests, etc. By the way, python3 -m pytest executes all tests and they must pass, of course. Thanks, Best, Hans

I did points 1 and 3. Regarding point 2, what do you mean by "turn your code on/off"? To have a variable that activates the code when desired?

Also, the multicore tests fail. I think it's because we consider an all-to-all architecture in all cases a multicore platform is used. Should it be changed to not consider an all-to-all architecture always and when calling the rEOO algorithm a prerequisite is to have an all-to-all architecture?

Best, Anabel.

Hopery commented 2 years ago

Hi Anabel,

  1. As far as I can see, I'm still missing an option to enable/disable your algorithm. Can you add it?
  2. I saw your example.py file. Can you please turn it into a file in tests, e.g. tests/test_chong.py that checks whether the chong partitioning implementation is working? You then have to intercept the output (e.g. in tests/test_output/test_chong_last.qasm, when you call the test internally in the python file test_chong) and add it to the qasm files in the directory golden.
  3. You updated the multi-core-4x4 json file from the one in tests and used by the other tests. Is that wise? If so, please give it another name to avoid confusion. I would prefer that you use the one in tests with 16 cores since the other multi-core tests use that one as well.
  4. And after all that do a run of python3 -m pytest that demonstrates that the other tests pass.

Thanks!

Hi Hans,

I just read your comment, I did a comment regarding point 2 and the tests, but everything is clear now.

Best, Anabel.