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

Triton ensemble export #2251

Merged
merged 65 commits into from
Aug 16, 2022
Merged

Triton ensemble export #2251

merged 65 commits into from
Aug 16, 2022

Conversation

abidwael
Copy link
Contributor

@abidwael abidwael commented Jul 11, 2022

Exports Triton configs and scripted models as well an an ensemble config.

  1. train a ludwig model.
from ludwig.api import LudwigModel
from ludwig.datasets import titanic

training_set, test_set, _ = titanic.load(split=True)
model = LudwigModel(config="./titanic.yaml")

train_stats, preprocessed_data, output_directory = model.train(training_set=training_set,
                                                               test_set=test_set,
                                                               experiment_name="simple_experiment",
                                                               model_name="simple_model",
                                                               skip_save_processed_input=True)
  1. To export to models and configs to a Triton-compliant structure
export_triton(model, data_example, output_path, model_name, model_version, device, device_count)
  1. Find the exported models under model_repository/

@abidwael abidwael requested a review from brightsparc July 11, 2022 01:16
Copy link
Contributor

@brightsparc brightsparc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abidwael This is looking good. I suggest you look to leverage the inference_utils which is being split out in this draft pr #2213 (perhaps you could land this first). So that you are not repeating the pre/post processing functions.

There will need to be some additional work to support images see example for model that includes text and image columns.

Would also be nice if we could have a way of providing triton InstanceGroup settings to be passed in, at a minimum cpu_count and gpu_count could be useful properties which we could then use herufistic

pre/post processing == cpu_count
predict = gpu_count if gpu_count > 0 else cpu_count

ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jul 11, 2022

Unit Test Results

       6 files  ±0         6 suites  ±0   2h 44m 41s ⏱️ - 3m 47s
2 966 tests ±0  2 914 ✔️  - 3    52 💤 +3  0 ±0 
8 898 runs  ±0  8 706 ✔️  - 9  192 💤 +9  0 ±0 

Results for commit e34d411. ± Comparison against base commit f654591.

♻️ This comment has been updated with latest results.

@abidwael abidwael force-pushed the triton-ensemble-export branch from 092f613 to 22f1564 Compare July 11, 2022 17:21
Copy link
Contributor

@brightsparc brightsparc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting there, I've added some comments.

I'd like to see a bit more flexibility added around the list of paths that we return so that we can support different file types in future, and pass along the content type when uploading to s3. This could be done in another PR if necessary.

We do need this PR to update integration tests and the cli export as well as fix any remaining tests.

ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@brightsparc brightsparc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; just add file size.

ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
ludwig/utils/triton_utils.py Outdated Show resolved Hide resolved
@abidwael abidwael merged commit ed8d9cf into master Aug 16, 2022
@abidwael abidwael deleted the triton-ensemble-export branch August 16, 2022 23:06
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 this pull request may close these issues.

2 participants