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

Record git revision during the installation #560

Merged
merged 24 commits into from Mar 11, 2024
Merged

Record git revision during the installation #560

merged 24 commits into from Mar 11, 2024

Conversation

grwlf
Copy link
Contributor

@grwlf grwlf commented Feb 28, 2024

By this PR we introduce and auto-generated catalyst._revision file containing __revision__ variable. When possible, this variable will contain latest known Git revision. In particular, users will be able to report the exact revisions by doing import catalyst; print(catalyst.__revision__) before submitting issues.

@grwlf grwlf marked this pull request as ready for review February 28, 2024 09:38
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.49%. Comparing base (57dbadf) to head (19125fd).

❗ Current head 19125fd differs from pull request most recent head 33d4cdf. Consider uploading reports for the commit 33d4cdf to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #560   +/-   ##
=======================================
  Coverage   99.49%   99.49%           
=======================================
  Files          50       50           
  Lines        8739     8745    +6     
  Branches      624      624           
=======================================
+ Hits         8695     8701    +6     
  Misses         25       25           
  Partials       19       19           

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

@dime10 dime10 changed the base branch from main to zne March 1, 2024 15:25
@dime10 dime10 changed the base branch from zne to main March 1, 2024 15:25
@erick-xanadu
Copy link
Contributor

@grwlf I like this! Do you think it would be a good idea to print this along with the version? Maybe a version() method? Along with the pennylane-lightning hash used to build the runtime?

@grwlf
Copy link
Contributor Author

grwlf commented Mar 11, 2024

Do you think it would be a good idea to print this along with the version? Maybe a version() method? Along with the pennylane-lightning hash used to build the runtime?

@erick-xanadu I think it would be nice indeed, but it would require discussions and so I'd prefer to skip it for now because I think it is important to have at least something.

The investigation is required to find out the "most standard" solution. The __revision__ I am proposing resembles this PEP 8 standard. The standard does not mention any __revision__, but the version() you are suggesting departs from it in a different way. The closest (but still non-standard UPD: no, actually it is kind of compliant) way I know would be to set __version__ to something like 1.2.3.dev+gAAAAAAA (see here). But can we include the second git revision from PL/L in the same manner? 1.2.3.dev+gAAAAA+gBBBBB? Might be, not sure. I am ready initiate this discussion in the channel right after we merge this bare minimum PR.

@grwlf grwlf requested a review from erick-xanadu March 11, 2024 15:05
@grwlf grwlf enabled auto-merge (squash) March 11, 2024 15:11
setup.py Outdated Show resolved Hide resolved
grwlf and others added 2 commits March 11, 2024 22:05
Co-authored-by: David Ittah <dime10@users.noreply.github.com>
@grwlf grwlf merged commit b818b55 into main Mar 11, 2024
34 checks passed
@grwlf grwlf deleted the record-git-revision branch March 11, 2024 18:28
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

3 participants