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

[Frontend] Improve readability of conditional passes #1194

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

erick-xanadu
Copy link
Contributor

Context: We are using the term pipeline a lot. We are also defining compilation pipelines at the module level and modifying them depending on configuration options.

Description of the Change: Change the compilation pipeline into stages and improve readability of passes that are conditional on compile time options.

Benefits: Readability.

@erick-xanadu erick-xanadu marked this pull request as ready for review October 9, 2024 15:21
Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.95%. Comparing base (75dc517) to head (f6b28ee).

Files with missing lines Patch % Lines
frontend/catalyst/debug/compiler_functions.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1194      +/-   ##
==========================================
+ Coverage   96.72%   97.95%   +1.23%     
==========================================
  Files          56       77      +21     
  Lines        6800    11331    +4531     
  Branches      780      978     +198     
==========================================
+ Hits         6577    11099    +4522     
- Misses        173      182       +9     
  Partials       50       50              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

doc/dev/debugging.rst Outdated Show resolved Hide resolved
@paul0403
Copy link
Contributor

paul0403 commented Oct 9, 2024

🥳 Thanks! Apart from readability I like how these lists are no longer just globals floating around.

Is it possible to increase some test coverage to address codecov?

doc/dev/debugging.rst Outdated Show resolved Hide resolved
@erick-xanadu
Copy link
Contributor Author

Is it possible to increase some test coverage to address codecov?

I am a bit opposed to creating unit tests for these functions. They are so simple and it would just make us recreate the list in the tests. This would mean having to modify tests whenever we change the list of passes to run.

@erick-xanadu erick-xanadu force-pushed the eochoa/2024-10-09/readability-of-compilation-pipeline branch from d81550f to 03da19f Compare November 7, 2024 12:56
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.

4 participants