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.

Sign up for GitHub

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

Jump to bottom

Work on adding CI to umd repo #45

Closed
broskoTT opened this issue Sep 6, 2024 · 5 comments · Fixed by #70, #71, #74, #87 or #88
Closed

Work on adding CI to umd repo #45

broskoTT opened this issue Sep 6, 2024 · 5 comments · Fixed by #70, #71, #74, #87 or #88
Assignees
@broskoTT
Labels
testing infrastructure Any kind of work which helps with testing or writing tests

Comments

@broskoTT
Copy link
Contributor

broskoTT commented Sep 6, 2024

Nothing to add for know, it should be pretty clear. Guard the UMD with tests
@broskoTT broskoTT added the enhancement New feature or request label Sep 6, 2024
@vtangTT
Copy link
Contributor

vtangTT commented Sep 9, 2024

Just FYI, as a stopgap solution for metal, we are running most/all UMD tests on the metal CI. They run for every push to main and every post-commit

https://github.com/tenstorrent/tt-metal/actions/workflows/umd-unit-tests-wrapper.yaml

@broskoTT broskoTT self-assigned this Sep 10, 2024
@broskoTT broskoTT added testing infrastructure Any kind of work which helps with testing or writing tests and removed enhancement New feature or request labels Sep 10, 2024
@broskoTT
Copy link
Contributor Author

Every push to tt-metal main, or umd main?
In any case we should have a CI here if we plan to ship this as a separate component/endpoint to our products.

@vtangTT
Copy link
Contributor

vtangTT commented Sep 10, 2024 •
edited
Loading

Every push to tt-metal main will run post-commit which includes umd tests. Example run from most recent main:

https://github.com/tenstorrent/tt-metal/actions/runs/10794191678/job/29937907086

But I agree, I think the CI should ultimately be in this repo.

@broskoTT
Copy link
Contributor Author

That seems unnecessary, since not every commit on tt-metal will update the UMD commit used. Sounds like you're running same tests on the same version of umd?
Also, this doesn't offer any guarantees for UMDs progress, i.e. this doesn't guarantee the current tip of the branch is functional.

@vtangTT
Copy link
Contributor

vtangTT commented Sep 10, 2024

Yes, I agree it's not the best solution and like you said it only guarantees umd is not broken for the version metal points to.

It was mainly implemented to have some sort of verification for UMD (since at that time we aren't able to get CI machines onto the UMD repo). Also on the off chance that metal exposes an edge-case bug in umd.

It also allowed us to dispatch umd tests on CI through this pipeline: https://github.com/tenstorrent/tt-metal/actions/workflows/umd-unit-tests-wrapper.yaml

The tests themselves run fast enough such that they don't affect the total runtime of post-commit. If development is ramping up on UMD, CI should def be present in this repo, but temporarily it's possible to validate through metal CI if needed.

This was referenced Sep 23, 2024
Merged
Merged
Merged
This was linked to pull requests Sep 25, 2024
Add CI for building and testing UMD #70
Merged
Create test-runner.yaml #71
Merged
Change compiler to clang #74
Merged
broskoTT added a commit that referenced this issue Sep 26, 2024
@broskoTT
Change compiler to clang (#74)
9b5f8cd 
Related to #45 
tt-metal uses clang compiler.
This configuration was copied from metal's CMakeLists.txt.
The irds that we're using for tt-metal already have this setup, so it
shouldn't disrupt anyone.
I've already installed clang-17 in #70, so CI will still work with this
switch.
This was linked to pull requests Oct 1, 2024
#86: Remove ARCH_NAME dependency for libdevice.so #87
Merged
No ARCH_NAME in CI building umd_device #88
Merged
No ARCH_NAME in CI building umd_device - on push job #89
Merged
Fix ownership issues with runner self-test #96
Merged
Enable grayskull on CI #97
Merged
@broskoTT broskoTT linked a pull request Oct 9, 2024 that will close this issue
Fix Grayskull tests #138
Merged
@broskoTT broskoTT mentioned this issue Nov 21, 2024
Merged
broskoTT added a commit that referenced this issue Nov 21, 2024
@broskoTT
Refactor our CI yamls (#319)
5e4f52a 
### Issue
No current issue, related to already closed #45 

### Description
As we're using CI for some time, I found that it actually brings
unnecessary complications to have a layer of yamls just designating what
is done on pr, optionally on pr, and on push. This can be directly added
to each of the jobs.

Another effect is that in the "Actions" tab, we won't have On PR,
Optional On PR, and On Push where each includes several different jobs.
We will now have Build device, build and run tests, and pre-commit ,
which will run both on PR branches and main.

### List of the changes
- Created a build-and-run-all-tests.yml which wraps build-tests and
run-tests for all archs. This just nicely corresponds exactly to
previous on-pr-opt.
- Deleted on-pr, on-pr-opt and on-push ymls
- Added to all referenced .ymls the config to run on pr and/or on push,
so that same jobs are called on same triggers.
- Added timeout parameter to a couple of jobs.
- I will change in settings which jobs are mandatory, if they are now
named slightly different.

### Testing
CI passes on this PR

### API Changes
There are no API changes in this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@broskoTT broskoTT

Labels
testing infrastructure Any kind of work which helps with testing or writing tests
Projects
None yet
Milestone
No milestone
2 participants
@broskoTT @vtangTT