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

Fix out of sync creation of NetQASM LAST log #29

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Conversation

bkreynen
Copy link
Contributor

@bkreynen bkreynen commented Jul 11, 2023

While working on QNE we found an issue in NetQASM/SquidASM (the issue is on the NetQASM repo QuTech-Delft/netqasm#55 ).

In short: the log files were being processed in both NetQASM and SquidASM due to which it happened out of order. To fix this a change was made to NetQASM (see PR: QuTech-Delft/netqasm#56 for a detailed explanation) where the log files are now copied in NetQASM instead. Although these changes in SquidASM are not strictly required (NetQASM will just overwrite the log files SquidASM puts in LAST). For cleanliness I removed the calls in SquidASM.

@bkreynen bkreynen force-pushed the fix/netqasm-last-log branch 2 times, most recently from 30106b7 to ea8c733 Compare July 11, 2023 15:59
@bkreynen
Copy link
Contributor Author

Not entirely sure why this pipeline is failing. I tested with a PR with 0 changes as well and the pipeline still failed

@bkreynen bkreynen requested a review from bvdvecht July 11, 2023 16:06
@bkreynen bkreynen marked this pull request as ready for review July 11, 2023 16:09
bvdvecht
bvdvecht previously approved these changes Jul 12, 2023
@bvdvecht bvdvecht dismissed their stale review July 12, 2023 08:07

Approved but then saw the examples pipeline failed.

@bvdvecht
Copy link
Collaborator

It looks like the issue with the example pipeline is related to the pydantic dependency version. The CI uses pydantic 2.X, which apparently breaks things that worked on 1.X. Changing setup.cfg such that pydantic is limited to <2.0 should fix this.
@bkreynen Could you add another commit with this small change?

@bkreynen
Copy link
Contributor Author

Thanks @bvdvecht I pushed a fix

@bvdvecht bvdvecht merged commit 2760e75 into develop Jul 13, 2023
4 checks passed
@bvdvecht bvdvecht deleted the fix/netqasm-last-log branch July 13, 2023 07:17
@bvdvecht
Copy link
Collaborator

Thanks @bkreynen ! Merged.

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.

None yet

2 participants