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

[Test] E2e Tests for Notebook Examples #2246

Closed
Electronic-Waste opened this issue Sep 2, 2024 · 10 comments
Closed

[Test] E2e Tests for Notebook Examples #2246

Electronic-Waste opened this issue Sep 2, 2024 · 10 comments

Comments

@Electronic-Waste
Copy link
Member

What you would like to be added?

We plan to add e2e tests for notebooks in CI/CD, run with papermill.

REF: https://github.com/nteract/papermill

Why is this needed?

This will help us ensure the correctness of our notebook examples for data scientists.

Love this feature?

Give it a 👍 We prioritize the features with most 👍

@andreyvelich
Copy link
Member

/remove-label lifeycycle/needs-triage
/area testing

Copy link

@andreyvelich: The label(s) /remove-label lifeycycle/needs-triage cannot be applied. These labels are supported: tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, lifecycle/needs-triage. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/remove-label lifeycycle/needs-triage
/area testing

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@andreyvelich
Copy link
Member

/remove-label lifecycle/needs-triage

@andreyvelich
Copy link
Member

/good-first-issue

Copy link

@andreyvelich:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@saileshd1402
Copy link
Contributor

/assign

@andreyvelich
Copy link
Member

cc @shravan-achar @akshaychitneni @bigsur0 for the Papermill usage in Kubeflow Training Operator.

@saileshd1402
Copy link
Contributor

saileshd1402 commented Sep 23, 2024

Hi All!
Regarding this issue I am facing a few blockers for some of the notebooks to run e2e tests on. I would like the community's opinions on the same:

  1. Fine-Tune-BERT-LLM.ipynb: This notebook requires GPUs and an AWS S3 Bucket. I am not aware if we can use GPUs in CI and what S3 Buckets we can use.
  2. train_api_hf_dataset.ipynb: I have faced the same problem as pointed out by this issue-Encountered an error while running the example in the document train_api_hf_dataset #2176. Should do the change suggested in the issue?
  3. train_api_s3_dataset.ipynb: I am not sure what to use as S3 bucket for this case in CI.

Due to these blockers on some of the files, I wanted to ask if the community thinks it's a good idea to raise separate PRs for each notebook? Please let me know your thoughts.

cc: @andreyvelich @Electronic-Waste @tenzen-y

@andreyvelich
Copy link
Member

This notebook requires GPUs and an AWS S3 Bucket. I am not aware if we can use GPUs in CI and what S3 Buckets we can use.

Let's post-pone the E2E for this Notebook. Maybe we can create a simple version of this example with just CPU and without S3 bucket requirement. Like how we did in this PR: #2199.
cc @helenxie-bit

Should do the change suggested in the issue?

Yes, just merged this PR.

train_api_s3_dataset.ipynb: I am not sure what to use as S3 bucket for this case in CI.

@kubeflow/wg-training-leads Any thoughts on how we can test the S3 in our CI ?

Due to these blockers on some of the files, I wanted to ask if the community thinks it's a good idea to raise separate PRs for each notebook? Please let me know your thoughts.

I think, just initially stepping the infrastructure and running just a single simple Notebook would be nice.

@andreyvelich
Copy link
Member

This has been resolved by: #2274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants