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

More timings #43

Merged
merged 4 commits into from
Mar 11, 2024
Merged

More timings #43

merged 4 commits into from
Mar 11, 2024

Conversation

peterfpeterson
Copy link
Member

@peterfpeterson peterfpeterson commented Mar 11, 2024

Since the larger problem with WAND^2 is that the queue is getting full, this add mores metrics which estimate the reduction time. Unfortunately, it is estimating the time for reduction as the difference between when mantid-framework is initialized and the last algorithm ends. Reduction scripts do not have to be written this way, but this gives a minimum time spent running the reduction.

Check list for the pull request

  • I have read the [CONTRIBUTING]
  • I have read the [CODE_OF_CONDUCT]
  • I have added tests for my changes
  • I have updated the documentation accordingly

Check list for the reviewer

  • I have read the [CONTRIBUTING]
  • I have verified the proposed changes
  • best software practices
    • all internal functions have an underbar, as is python standard
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date
  • code comments added when explaining intent

Manual test for the reviewer

References

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 82.22222% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 77.17%. Comparing base (e367658) to head (48a6342).
Report is 1 commits behind head on main.

Files Patch % Lines
scripts/ar_report.py 82.22% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
- Coverage   82.93%   77.17%   -5.76%     
==========================================
  Files          15       16       +1     
  Lines         920     1240     +320     
==========================================
+ Hits          763      957     +194     
- Misses        157      283     +126     

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

@peterfpeterson peterfpeterson requested a review from backmari March 11, 2024 14:01
@backmari
Copy link
Collaborator

The code changes look reasonable. I am fine ignoring the codecov "error".

I tested the script on an autoreduced HB2C run and can see in the report that the new metric meas-redux shows that the reduction took longer than the run.
image

@peterfpeterson peterfpeterson merged commit e2b2459 into main Mar 11, 2024
4 of 5 checks passed
@peterfpeterson peterfpeterson deleted the more_timings branch March 11, 2024 14:57
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.

2 participants