Navigation Menu

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

Understanding the CI #2640

Closed
rbharath opened this issue Aug 4, 2021 · 1 comment · Fixed by #2713
Closed

Understanding the CI #2640

rbharath opened this issue Aug 4, 2021 · 1 comment · Fixed by #2713

Comments

@rbharath
Copy link
Member

rbharath commented Aug 4, 2021

The CI system is really complex to understand right now. I'm typing up some notes here to help me and the other reviewers so we can better keep track of code state. This should also be useful to new developers so they can self-check PRs. We should merge these notes into the docs later.

The CI has several stages of checks:

  1. Build: These are the absolute basic tests that check if DeepChem can be installed and imported after the PR is merged. If these tests fail, there's something terribly wrong and the PR absolutely shouldn't be merged.
  2. Core: This should be renamed something like "Developer tools test". It consists of flake8/doctest/mypy checks. These checks should also not fail in a PR. We've been bad about keeping track of this but I'll try to fix this moving forward. If PR fails Core tests, it shoudn't be merged in.
  3. Common: These are all the non backend (that is no Jax/PyTorch/TF usage) tests. These should also pass most of the time.
  4. Jax/Torch/TensorFlow: These different runs respectively only test Jax/Torch/TensorFlow tests. Most flaky tests will reside in one of these runs and have to do something with a numerical issue from a deep learning backend. It may be OK to merge a PR even if it fails a test here and we have some lingering breakages here we're trying to fix (the challenge is that each backend version bump introduces slight numerical changes which can break our correctness tests)
@arunppsg
Copy link
Contributor

Renaming core will be useful as the name causes confusion with build sometimes.

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 a pull request may close this issue.

2 participants