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

rOEE mapping algorithm implementation. #442

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

Hopery
Copy link

@Hopery Hopery commented Mar 29, 2022

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

@jvansomeren
Copy link
Collaborator

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
Copy link
Author

Hopery commented Mar 29, 2022

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
Copy link
Author

Hopery commented Mar 29, 2022

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.

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