-
Notifications
You must be signed in to change notification settings - Fork 16
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
Lightsout updates #227
base: main
Are you sure you want to change the base?
Lightsout updates #227
Conversation
# Conflicts: # pyproject.toml # src/mqt/qecc/cc_decoder/decoder.py # src/mqt/qecc/cc_decoder/plotting/plots.py
…t-updates # Conflicts: # src/mqt/qecc/cc_decoder/stim_interface/color_code_stim.py
Signed-off-by: burgholzer <burgholzer@me.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on this 🙏🏼
I quickly went through everything and adjusted a couple of things myself.
Most importantly, I moved all of the evaluation code outside of the main package and into the scripts directory. Files like that typically do not belong in the package itself.
Besides the inline comments below, this PR is completely missing tests.
None of the stim integration is tested, which makes it quite hard to figure out whether it is actually working as intended.
If the stim interface should be part of the package (which probably makes sense), it should be fully tested to ensure future compatibility.
I'd also advise to re-run all the plotting scripts and other evaluation scripts at least once after all changes have been incorporated in order to check whether they still work.
Hopefully, that does not consume too much time 😌
src/mqt/qecc/cc_decoder/stim_interface/max_sat_sinter_decoder.py
Outdated
Show resolved
Hide resolved
# Conflicts: # pyproject.toml # uv.lock
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your iteration on the code! Almost done.
Please try to get the CI to an all-green state. The sinter
part of this PR is not yet covered by any tests. I am pretty sure you will notice that something is missing once you start writing the tests ;-)
Also, please do not needlessly add linter ignores. Especially not when the linter already tells you precisely what to do/fix and it's just a hand full of warnings to address.
Did that now in 330830c.
# Conflicts: # pyproject.toml # uv.lock
Description
Includes recent updates for the liightsout decoder and stim integration.
Checklist: