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

Restructuring unit tests with their own sub-dirs #70

Merged
merged 36 commits into from Jun 8, 2022
Merged

Conversation

nariman87
Copy link
Contributor

@nariman87 nariman87 commented Jun 1, 2022

Context for changes

  • Improvements:
    • Unit tests have been re-grouped in individual sub-dirs inside tests/ based on error correction and software layers. This helps manage and target each test unit.
    • build_tests.yaml workflow now supports executing unit tests in parallel using pytest-xdist package. GitHub runners have at least 2 processors, which helps speed up the pytest blocks by ~1.5 times in practice.

Example usage and tests

  • Not Applicable

Performance results justifying changes

  • Not Applicable

Workflow actions and tests

See the updated .github/workflows/build_tests.yaml.

Expected benefits and drawbacks

Expected benefits:

  • unit tests now have their own sub-dirs based on error correction and software layers and are faster to run thanks to xdist parallelization.

Related Github issues

  • Not Applicable

Checklist and integration statements

  • My Python and C++ codes follow this project's coding and commenting styles as indicated by existing files. Precisely, the changes conform to given black, docformatter and pylint configurations.
  • I have performed a self-review of these changes, checked my code (including for codefactor compliance), and corrected misspellings to the best of my capacity. I have synced this branch with others as required.
  • I have added context for corresponding changes in documentation and README.md as needed.
  • I have added new workflow CI tests for corresponding changes, ensuring codecoverage is 95% or better, and these pass locally for me.
  • I have updated CHANGELOG.md following the template. I recognize that the developers may revisit CHANGELOG.md and the versioning, and create a Special Release including my changes.

@nariman87 nariman87 added the workflow & tests CI tests have been modified and/or new workflows were added label Jun 1, 2022
@nariman87 nariman87 self-assigned this Jun 1, 2022
@nariman87 nariman87 marked this pull request as draft June 1, 2022 11:30
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #70 (0383e7e) into main (1363792) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #70   +/-   ##
=======================================
  Coverage   96.28%   96.29%           
=======================================
  Files          36       36           
  Lines        2233     2238    +5     
=======================================
+ Hits         2150     2155    +5     
  Misses         83       83           
Impacted Files Coverage Δ
flamingpy/codes/surface_code.py 97.31% <ø> (ø)
flamingpy/decoders/decoder.py 98.42% <ø> (ø)
flamingpy/decoders/unionfind/algos.py 99.27% <ø> (ø)
flamingpy/decoders/unionfind/uf_classes.py 99.08% <ø> (ø)
flamingpy/examples/decoding.py 100.00% <ø> (ø)
flamingpy/utils/viz.py 90.22% <ø> (ø)
flamingpy/_version.py 100.00% <100.00%> (ø)
flamingpy/cv/gkp.py 92.85% <100.00%> (+0.12%) ⬆️
flamingpy/cv/macro_reduce.py 95.68% <100.00%> (ø)
flamingpy/cv/ops.py 91.66% <100.00%> (+0.09%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1363792...0383e7e. Read the comment docs.

@nariman87 nariman87 changed the title Restructuring workflow tests with their own units Restructuring unit tests with their own sub-dirs Jun 3, 2022
@nariman87 nariman87 marked this pull request as ready for review June 3, 2022 03:03
Copy link
Contributor

@sduquemesa sduquemesa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @nariman87, it does make the testing suite much more organized and focused. Overall it looks great! I have left some comments and question, in the meantime perhaps other code-owners can review the PR as well.

.github/CHANGELOG.md Show resolved Hide resolved
.github/workflows/linting.yaml Outdated Show resolved Hide resolved
flamingpy/utils/viz.py Outdated Show resolved Hide resolved
.github/workflows/build_tests.yaml Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
tests/cv/test_gkp.py Show resolved Hide resolved
doc/index.rst Show resolved Hide resolved
tests/cv/test_gkp.py Show resolved Hide resolved
nariman87 and others added 2 commits June 4, 2022 12:00
Co-authored-by: Sebastián Duque Mesa <675763+sduquemesa@users.noreply.github.com>
@nariman87 nariman87 requested a review from sduquemesa June 4, 2022 05:23
Copy link
Contributor

@sduquemesa sduquemesa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nariman87 for addressing all the comments! ✨ Let's see what are @ilan-tz's opinions on the quick-install/install issue before merging.

Copy link
Collaborator

@ilan-tz ilan-tz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great - an important PR with several nice improvements. Just have a few small questions.

EDIT: Saw previous discussion! All good.

flamingpy/cv/macro_reduce.py Show resolved Hide resolved
tests/cv/test_macro_reduce.py Outdated Show resolved Hide resolved
tests/examples/test_examples.py Show resolved Hide resolved
.github/workflows/linting.yaml Show resolved Hide resolved
@ilan-tz ilan-tz merged commit 9d50fe0 into main Jun 8, 2022
@ilan-tz ilan-tz deleted the restructuring-tests branch June 8, 2022 17:58
soosub pushed a commit that referenced this pull request Jul 29, 2022
* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update CMakeLists.txt

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update setup.py

* Update setup.py

* Update CMakeLists.txt

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Creating a dedicated dir for fast_ec_mc ...

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update setup.py

* Update CMakeLists.txt

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update CMakeLists.txt

* Update setup.py

* Update setup.py

* Update setup.py

* Update CMakeLists.txt

* Update fast_ec_mc.pyx

* Update fast_ec_mc.pyx

* Update simulations.py

* Update simulations.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Update fast_ec_mc.pyx

* Update setup.py

* Update setup.py

* Update fast_ec_mc.pyx

* Update setup.py

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Some restructuring ...

* Update setup.py

* Update simulations.py

* Update fast_ec_mc.pyx

* Update setup.py

* Update setup.py

* Update setup.py

* Update fast_ec_mc.pyx

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Adding sims_data ...

* Delete results.csv

* Delete 1645632889_simulations_results.csv

* Delete 1645635881_simulations_results.csv

* Update simulations.py

* Update fast_ec_mc.pyx

* Update fast_ec_mc.pyx

* Update and rename fast_ec_mc.pyx to fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update simulations.py

* Update setup.py

* Update fast_mc_loop.pyx

* Update simulations.py

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update simulations.py

* Update simulations.py

* Update setup.py

* Update simulations.py

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Update simulations.py

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update simulations.py

* Update fast_mc_loop.pyx

* Update simulations.py

* Update simulations.py

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update README.md

* Update build_tests.yaml

* Update linting.yaml

* Update upload_linux_x64.yaml

* Update upload_macos_x64.yaml

* Update upload_win_x64.yaml

* Update docs.yaml

* Update simulations.py

* Update simulations.py

* Update fast_mc_loop.pyx

* Update fast_mc_loop.pyx

* Update results.csv

* Update simulations.py

* Update MANIFEST.in

* Update .pylintrc

* Update lemon.py

* Update test_simulations.py

* Update test_simulations.py

* Update test_simulations.py

* Update results.csv

* Update README.md

* Update README.md

* Update simulations.py

* Update simulations.py

* Update test_simulations.py

* Update simulations.py

* Update simulations.py

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Some restructuring ...

* Update setup.py

* Update MANIFEST.in

* Update CMakeLists.txt

* Update simulations.py

* Update setup.py

* Rename decoder.py to decoding.py

* Create simulations.py

* Update and rename fast_mc_loop.pyx to cpp_mc_loop.pyx

* Update setup.py

* Update simulations.py

* Update simulations.py

* Update MANIFEST.in

* Update test_simulations.py

* Update test_benchmarks_examples.py

* Update simulations.py

* Update simulations.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update setup.py

* Update test_threshold_plots.py

* Update lemon.py

* Update test_benchmarks_examples.py

* Update format.yaml

* Update _version.py

* Some black reformatting ...

* Update README.md

* Update results files, directories

* Update flamingpy/simulations.py

Co-authored-by: ilan-tz <57886357+ilan-tz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow & tests CI tests have been modified and/or new workflows were added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants