-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add a gpu sample #575
Add a gpu sample #575
Conversation
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This was a sample test change. Did you verify that the sample test succeeded? |
@@ -51,6 +51,11 @@ def kubeflow_tf_training_op(transformed_data_dir, schema: 'GcsUri[text/json]', l | |||
], | |||
file_outputs = {'train': '/output.txt'} | |||
) | |||
if use_gpu: | |||
kubeflow_tf_training_op.image = 'gcr.io/ml-pipeline/ml-pipeline-kubeflow-tf-trainer-gpu:85c6413a2e13da4b8f198aeac1abc2f3a74fe789', |
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.
.image
needs to be an string, not tuple.
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.
nice catch
@gaoning777 I think we still need to run sample test whenever someone modifies a sample. It should probably test just that sample. |
Sample tests are run in the backend no matter what. Are you inferring that we should enforce the sample test passing for merging in case samples are updated? |
* Define a script to close obsolete PRs to update an application. * In the event there are multiple PRs open to update a Kubeflow application we want to close the older PRs; so there is a single open PR updating the application to the newest code. Related to kubeflow#571 * Define a script to close obsolete PRs to update an application. * In the event there are multiple PRs open to update a Kubeflow application we want to close the older PRs; so there is a single open PR updating the application to the newest code. Related to kubeflow#571 * Setup a def namespace for use with apps-cd. * Update update_kf_apps.py to close old PRs on each sync. * Bake the source code into the docker image rather than using a wrapper script to sync the code from git. * Sync'ing the code from git became to difficult to reason about once we start splitting the source code across multiple repositories * We now depend on github/kubeflow/code-intelligence for utilities for working with GitHub Apps * Using a docker image also ensures we don't get broken suddenly when new changes are in place * In the future we could use github actions to automate updating the deployment on postsubmits * Turn app-pipeline.template.yaml into a ConfigMap * This allows better versioning * We can rely on kustomize to create a configmap with a hash based on the contents * kustomize will then reference the config map using its hash. As a result a rolling update is triggered whenever the hash contents changes. * This makes it easier to handle rollous and updates. Define a dev instance of the update KF apps infrastructure to facilitate development * Use profiles in skaffold. * update_kf_apps.py in dev uses a config map now to ubtain app-pipeline.template.yaml rather than fetching it from git * This makes it much easier to test changes in the dev instance Fix a bunch of bugs preventing update_kf_apps.py from working * Update requirements.txt with a bunch of missing packages. * Fix some imports in update_kf_apps.py * Need to set resource requests for the build pods otherwise builds get CPU starved and take forever. Miscellaneous * Create a tool to copy secrets between namespaces from GCS * Fix lint. * Due to kubeflow#460 we need to disable pylint.
Add logger in test configmap since it's already built for that
* update release manifests and openshift docs * update release manifests and openshift docs * add optional kustomization.yaml
This change is