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

adding an atime test case for gshift as gforce optimized shift #5205 #6295

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Copy link

github-actions bot commented Jul 17, 2024

Comparison Plot

Generated via commit 7a25d4f

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 3 minutes and 14 seconds

Time taken to run atime::atime_pkg on the tests: 7 minutes and 42 seconds

@tdhock tdhock marked this pull request as draft July 18, 2024 01:38
@tdhock
Copy link
Member

tdhock commented Jul 18, 2024

Hi I converted to draft because I think you should first merge your first PR, and then use what you learn from that experience to improve your other PRs, before asking others to review them.

@DorisAmoakohene
Copy link
Contributor Author

Hi I converted to draft because I think you should first merge your first PR, and then use what you learn from that experience to improve your other PRs, before asking others to review them.

I have made most corrections to this PR for review and merge

@DorisAmoakohene DorisAmoakohene marked this pull request as ready for review July 29, 2024 21:15
@tdhock
Copy link
Member

tdhock commented Jul 30, 2024

please resolve conflicts and fix typo shif -> shift

@DorisAmoakohene
Copy link
Contributor Author

please resolve conflicts and fix typo shif -> shift

I have resolved this. Thank you

.ci/atime/tests.R Outdated Show resolved Hide resolved
@MichaelChirico MichaelChirico added the atime Requests related to adding/improving performance regression tests via atime label Sep 9, 2024
Anirban166

This comment was marked as outdated.

@tdhock
Copy link
Member

tdhock commented Sep 14, 2024

currently we see this result
image

there are three issues

  1. there is not a huge time separation between Regression and Fixed at largest N, but there is some separation at smaller N, why? Can you please investigate, perhaps with a larger N? and double check the historical commit IDs?
  2. There is no Before, so please either add that, or change to Fast and Slow.
  3. why does Regression use less memory than Fixed?

@DorisAmoakohene
Copy link
Contributor Author

currently we see this result image

there are three issues

  1. there is not a huge time separation between Regression and Fixed at largest N, but there is some separation at smaller N, why? Can you please investigate, perhaps with a larger N? and double check the historical commit IDs?
  2. There is no Before, so please either add that, or change to Fast and Slow.
  3. why does Regression use less memory than Fixed?

Alright

@DorisAmoakohene
Copy link
Contributor Author

currently we see this result image

there are three issues

  1. there is not a huge time separation between Regression and Fixed at largest N, but there is some separation at smaller N, why? Can you please investigate, perhaps with a larger N? and double check the historical commit IDs?
  2. There is no Before, so please either add that, or change to Fast and Slow.
  3. why does Regression use less memory than Fixed?

I investigated further with a larger sample size (N), but found no difference, it stops when it gets to 10000

. Even when I reverted to the commit to the commit before the last commits, where the PR was merge nothing changes.

pic5205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
atime Requests related to adding/improving performance regression tests via atime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants