-
Notifications
You must be signed in to change notification settings - Fork 16
Adds initial CI suport #52
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're a hero for adding this. Just make sure the CI actually runs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll come back to this and fix when higher priority. Right now I don't consider this a P0.
Blocking b/c there should be a signal in CI before landing, therefore something isn't working.
This PR now runs all of the tests on CPU! please take another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some cleanup - thanks for this!
.pre-commit-config.yaml
Outdated
args: ['--branch=main'] | ||
- id: check-added-large-files | ||
args: ['--maxkb=1000'] | ||
# - id: check-added-large-files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remove if not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing this because pip install torchnightly
doesn't work... I know the right thing is to fix that, but I hope you understand that will be difficult for a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it - can you just refer to it in the pyproject.toml as e.g. "mylib @ file:///${PROJECT_ROOT}/dist/mylib-0.1.0-py3-none-any.whl"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is mostly because the pyproject.toml uses it, similar to torchtitan does it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But does it need it to run? IIUC, it should not and I'd prefer not to add until necessary.
This PR