fix benchmark action #2656

officialasishkumar · 2024-06-13T16:55:38Z

📝 Description

Type: 🪲 🚀

Fix github actions for benchmarks
Add a webpage for every PR that leads to a benchmark commit graph (eg: https://tardis-sn.github.io/tardis-benchmarks/#benchmark_run_tardis.Benchmarkruntardis.time_run_tardis)

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

tardis-bot · 2024-06-13T16:56:48Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

tardis-bot · 2024-06-13T17:48:20Z

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (e88a31a) and the latest commit (5d5e93c).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.
Significantly changed benchmarks:

· No results found

All benchmarks:

· No results found

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com> test commit

.gitignore

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

andrewfullard · 2024-06-21T14:24:39Z

Please test by making a version on your fork and add link to test results here

officialasishkumar · 2024-06-28T01:37:52Z

Please test by making a version on your fork and add link to test results here

https://github.com/AkashKumar7902/tardis/actions/runs/9705432873/job/26787490302?pr=6

the error in the later half is due to the generation of file which is relying upon tardis repo. The compare is working which means the bot will display the comparision output.

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

andrewfullard · 2024-07-01T14:01:23Z

.github/workflows/benchmarks.yml

@@ -80,6 +85,9 @@ jobs:
      - name: Install asv
        run: pip install asv

+      - run: git remote add upstream https://github.com/tardis-sn/tardis


Commands have no label

andrewfullard · 2024-07-01T14:02:06Z

.github/workflows/benchmarks.yml


      - name: Compare Master and PR head
-        run: asv compare ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} | tee asv-compare-output.log
+        run: asv compare upstream/master HEAD --config asv.conf.json | tee asv-compare-output.log


This is hardcoded

Unfortunately, the method of fetching the hash without hardcoding the branch hasn't been successful. Given that the master branch name is unlikely to change, would it be acceptable to use a hardcoded approach in this case?

Yes, I suppose so

jvshields · 2024-07-01T14:06:33Z

This may be helpful - https://stackoverflow.com/questions/61393981/getting-base-branch-sha-on-pull-request-in-github-action-workflow

officialasishkumar · 2024-07-01T14:12:58Z

https://github.com/sympy/sympy/blob/master/.github/workflows/runtests.yml

cc: @andrewfullard @atharva-2001

atharva-2001 · 2024-07-01T14:13:27Z

Yes, that's an extra step. I have removed it.

here is the new action run: officialasishkumar/tardis/actions/runs/9733397076/job/26860263830?pr=12

This workflow used this workflow file-
https://github.com/officialasishkumar/tardis/actions/runs/9733397076/workflow?pr=12
which does not have your commits. Is the workflow being correctly tested?
Note that these are pull request target workflows, so they will only pick the workflow file from the target(master) branch, not from the pull request branch.

officialasishkumar · 2024-07-01T14:31:37Z

Note that these are pull request target workflows, so they will only pick the workflow file from the target(master) branch, not from the pull request branch.

Here is the new workflow run: https://github.com/officialasishkumar/tardis/actions/runs/9746049892/job/26895430197?pr=13
Here is the master branch benchmarks.yml that was updated yesterday: https://github.com/officialasishkumar/tardis/blob/master/.github/workflows/benchmarks.yml

atharva-2001 · 2024-07-01T14:36:41Z

The workflow runs on this commit-
https://github.com/officialasishkumar/tardis/tree/bff68859d97a0764e38e5e598c48940e748e45a9

Not the master branch as far as I understand.

Click on the workflow file on the left side of this page-
https://github.com/officialasishkumar/tardis/actions/runs/9746049892/job/26895430197?pr=13

then click on commit hash, then browse files, and you will see this
https://github.com/officialasishkumar/tardis/tree/bff68859d97a0764e38e5e598c48940e748e45a9

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

officialasishkumar · 2024-07-01T14:43:15Z

@atharva-2001, the workflow file might be because my branch is not really updated with master for a while (approx 2 months). I think the workflow is running with the current master branch as you can see in the build that the jobs that are runned are aligned with the current changes and not the old one.

.github/workflows/benchmarks.yml

asv.conf.json

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

andrewfullard

I think we should try and merge this before we worry about solving the crashing individual benchmarks in #2640

atharva-2001 · 2024-07-03T13:34:05Z

.github/workflows/benchmarks.yml

+      - name: Fetch from upstream master
+        run: |
+          git remote add upstream https://github.com/tardis-sn/tardis
+          git fetch upstream master


this is what the checkout does right? Perhaps you could just do git remote add upstream URL?

what do you mean? git remote add upstream url is already present.

The first step just does this, perhaps the remote is origin there. If yes then we can perhaps reduce this step here.

doesn't work.

https://github.com/officialasishkumar/tardis/actions/runs/9779319538/job/26998276265?pr=46

By default I think it is origin, so it does not find upstream, just rename origin to upstream if you don;t want origin

updated

new action run: https://github.com/officialasishkumar/tardis/actions/runs/9783169581/job/27011135418?pr=48
workflow file: https://github.com/officialasishkumar/tardis/blob/master/.github/workflows/benchmarks.yml

.github/workflows/benchmarks.yml

atharva-2001 · 2024-07-03T13:44:20Z

.github/workflows/benchmarks.yml

+      - name: Fetch from upstream master
+        run: |
+          git remote add upstream https://github.com/tardis-sn/tardis
+          git fetch upstream master


The first step just does this, perhaps the remote is origin there. If yes then we can perhaps reduce this step here.

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

atharva-2001

Great work!

andrewfullard

If Atharva is happy, I am also happy. Cross your fingers!

fix benchmark action

74a46a2

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

officialasishkumar changed the title ~~fix benchmark action~~ [WIP] fix benchmark action Jun 13, 2024

andrewfullard added the benchmarks label Jun 13, 2024

officialasishkumar force-pushed the github-actions-benchmark branch 11 times, most recently from 600108c to cbefb9e Compare June 14, 2024 13:57

Push benchmark of every commit to tardis-benchmarks repository

abeeb4f

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com> test commit

officialasishkumar force-pushed the github-actions-benchmark branch from 5b135bc to abeeb4f Compare June 17, 2024 10:13

officialasishkumar changed the title ~~[WIP] fix benchmark action~~ fix benchmark action Jun 18, 2024

atharva-2001 reviewed Jun 20, 2024

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

officialasishkumar added 2 commits June 21, 2024 15:09

removed PR-*

fcc5ef3

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

update master to upstream/master in asv compare

e74980d

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

DeerWhale requested review from andrewfullard, afloers, DeerWhale and AlexHls June 25, 2024 12:39

officialasishkumar force-pushed the github-actions-benchmark branch 2 times, most recently from 5dcaa32 to aea28c5 Compare June 28, 2024 01:36

officialasishkumar force-pushed the github-actions-benchmark branch from a67d5b9 to e5d00a8 Compare June 30, 2024 15:14

checkout upstream

de3533b

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

officialasishkumar force-pushed the github-actions-benchmark branch from e5d00a8 to de3533b Compare June 30, 2024 15:46

fixed action run

033bbf1

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

andrewfullard reviewed Jul 1, 2024

View reviewed changes

name for fetch upstream

28a9816

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

atharva-2001 requested changes Jul 2, 2024

View reviewed changes

officialasishkumar mentioned this pull request Jul 2, 2024

remove github action workflow tardis-sn/tardis-benchmarks#4

Merged

add github pages for every PR

1d6fdc2

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

officialasishkumar force-pushed the github-actions-benchmark branch from 43c0849 to 1d6fdc2 Compare July 3, 2024 06:08

officialasishkumar added 3 commits July 3, 2024 14:38

setup lfs

b1a29fa

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

keep files during deploy

15c6e42

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

add bot_token

49c1bd0

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

andrewfullard previously approved these changes Jul 3, 2024

View reviewed changes

andrewfullard requested a review from atharva-2001 July 3, 2024 13:07

atharva-2001 reviewed Jul 3, 2024

View reviewed changes

atharva-2001 requested changes Jul 3, 2024

View reviewed changes

remove fetch job

5d5e93c

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>

officialasishkumar dismissed andrewfullard’s stale review via 5d5e93c July 3, 2024 18:47

andrewfullard self-requested a review July 3, 2024 20:22

atharva-2001 approved these changes Jul 4, 2024

View reviewed changes

atharva-2001 mentioned this pull request Jul 5, 2024

Fix missing atom data error in benchmark action #2674

Closed

6 tasks

andrewfullard approved these changes Jul 5, 2024

View reviewed changes

andrewfullard merged commit 97cf284 into tardis-sn:master Jul 5, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix benchmark action #2656

fix benchmark action #2656

officialasishkumar commented Jun 13, 2024 •

edited

Loading

tardis-bot commented Jun 13, 2024

tardis-bot commented Jun 13, 2024 •

edited

Loading

andrewfullard commented Jun 21, 2024 •

edited

Loading

officialasishkumar commented Jun 28, 2024

andrewfullard Jul 1, 2024

officialasishkumar Jul 1, 2024

andrewfullard Jul 1, 2024

officialasishkumar Jul 1, 2024

andrewfullard Jul 1, 2024

jvshields commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

atharva-2001 commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

atharva-2001 commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

andrewfullard left a comment

atharva-2001 Jul 3, 2024

officialasishkumar Jul 3, 2024

atharva-2001 Jul 3, 2024

officialasishkumar Jul 3, 2024

atharva-2001 Jul 3, 2024

officialasishkumar Jul 3, 2024 •

edited

Loading

atharva-2001 Jul 4, 2024

atharva-2001 Jul 3, 2024

atharva-2001 left a comment

andrewfullard left a comment

fix benchmark action #2656

fix benchmark action #2656

Conversation

officialasishkumar commented Jun 13, 2024 • edited Loading

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

tardis-bot commented Jun 13, 2024

tardis-bot commented Jun 13, 2024 • edited Loading

andrewfullard commented Jun 21, 2024 • edited Loading

officialasishkumar commented Jun 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jvshields commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

atharva-2001 commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

atharva-2001 commented Jul 1, 2024

officialasishkumar commented Jul 1, 2024

andrewfullard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

officialasishkumar Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atharva-2001 left a comment

Choose a reason for hiding this comment

andrewfullard left a comment

Choose a reason for hiding this comment

officialasishkumar commented Jun 13, 2024 •

edited

Loading

tardis-bot commented Jun 13, 2024 •

edited

Loading

andrewfullard commented Jun 21, 2024 •

edited

Loading

officialasishkumar Jul 3, 2024 •

edited

Loading