[REVIEW] Switch Models to use Crossfit #58

VibhuJawa · 2024-05-09T06:00:27Z

This PR enables using Crossfit.

Todo:

Domain Model
Quality Model
Update installation instructions (Verified locally)

Benchmarks:

Subset Place	Dataset Size	Batch Size	GPUs	Time Taken	Implementation	Speedup	Note
subset_CC-MAIN-2023-14_english	1.8 GB	1024** (dynamic)	16 V100	163	Crossfit	1	Batch size is dynamic
subset_CC-MAIN-2023-14_english	1.8 GB	256 (static)	16 V100	230	MainLine	1.41	Batch size is static, 512,1024 both OOM

A100 numbers:

Dataset	Size (Mb)	Resolution	GPU	Time (s)	Model	Speedup	Batch Size
subset_CC-MAIN-2023-14_english	148	1024**	2 A100	50.558	Crossfit	1	Batch size is dynamic
subset_CC-MAIN-2023-14_english	148	1024**	2 A100	78.487	Mainline	1.55	Batch size is static

VibhuJawa · 2024-05-09T08:50:06Z

CC: @sarahyurick for an initial review.

ayushdg · 2024-05-09T18:28:09Z

Side note: NeMo-Curator requires all PR's to include both signed commits as well as signed-off. This can be enabled with git commit -sS ....
There's some more info in https://github.com/NVIDIA/NeMo-Curator/blob/main/CONTRIBUTING.md#pull-requests-pr-guidelines.

VibhuJawa · 2024-05-09T18:32:51Z

Side note: NeMo-Curator requires all PR's to include both signed commits as well as signed-off. This can be enabled with git commit -sS .... There's some more info in https://github.com/NVIDIA/NeMo-Curator/blob/main/CONTRIBUTING.md#pull-requests-pr-guidelines.

Thanks, fixed it.

nemo_curator/distributed_data_classification/domain_classifier_inference.py

sarahyurick

I haven't been able to try running this yet (conda environments are acting up so I'm running an extensive cleanup right now) but wanted to leave at least an initial review for now. Changes look good and very straightforward to me! I don't have any major concerns or issues with this; I plan to approve/request further changes as needed later today when I actually run it.

Only question atm is whether you're planning to add the quality classifier in this PR or sometime later? Either way is ok with me.

examples/distributed_data_classification_examples/domain_api_example.py

nemo_curator/distributed_data_classification/domain_classifier_inference.py

nemo_curator/modules/distributed_data_classifier.py

nemo_curator/distributed_data_classification/domain_classifier_inference.py

sarahyurick

I was able to run this as is! Thanks again.

examples/distributed_data_classification_examples/domain_api_example.py

VibhuJawa · 2024-05-16T23:21:47Z

examples/distributed_data_classification_examples/test_models.ipynb

@@ -0,0 +1,373 @@
+{


I can move this file if we don't like having notebooks but personally i think for examples notebooks are a better way to understand.

We can keep the notebook, but could you please move it to the tutorials folder? The name of the folder could be distributed_data_classification and the name of the notebook could be distributed_data_classification.ipynb.

Actually while we're moving stuff around, do you mind also moving everything in distributed_data_classification_examples to the root examples folder? I think they should be with everything else. And rename the files too please.

domain_api_example.py -> domain_classifier_example.py

quality_api_example.py -> quality_classifier_example.py

VibhuJawa · 2024-05-16T23:24:08Z

@sarahyurick , I have added the quality model and cleaned up scripts, Can i get a re-review please.

ryantwolf

I'm super glad to see a speedup like this, and thanks so much for removing a lot of code. I left a lot of nitpicks, and a couple of additional things I'd like to see cleaned up. Thanks for doing this!

ryantwolf · 2024-05-17T16:39:35Z

examples/distributed_data_classification_examples/test_models.ipynb

@@ -0,0 +1,373 @@
+{


We can keep the notebook, but could you please move it to the tutorials folder? The name of the folder could be distributed_data_classification and the name of the notebook could be distributed_data_classification.ipynb.

nemo_curator/distributed_data_classification/domain_classifier_inference.py

nemo_curator/distributed_data_classification/quality_classifier_inference.py

setup.py

examples/distributed_data_classification_examples/test_models.ipynb

ryantwolf · 2024-05-17T17:37:49Z

examples/distributed_data_classification_examples/test_models.ipynb

@@ -0,0 +1,373 @@
+{


Actually while we're moving stuff around, do you mind also moving everything in distributed_data_classification_examples to the root examples folder? I think they should be with everything else. And rename the files too please.

domain_api_example.py -> domain_classifier_example.py

quality_api_example.py -> quality_classifier_example.py

examples/quality_classifier_example.py

nemo_curator/distributed_data_classification/quality_classifier_multiple_models_inference.py

VibhuJawa · 2024-05-20T21:20:42Z

Really sorry for the commit noise guys (@ryantwolf , @sarahyurick ), i messed up one of the git rebase (forgot to sign off a commit).

This should be ready for another review now. Have addressed all the reviews and added issues to follow up ones that i could not get to in this PR.

ryantwolf

Great, I missed one file in my initial review, but other than that mostly just nits. Thanks!

tutorials/distributed_data_classification/distributed_data_classification.ipynb

nemo_curator/scripts/domain_classifier_inference.py

nemo_curator/scripts/classifier_arg_utils.py

nemo_curator/modules/distributed_data_classifier.py

Signed-off-by: Vibhu Jawa <[email protected]>

…ssification.ipynb Co-authored-by: Ryan Wolf <[email protected]> Signed-off-by: Vibhu Jawa <[email protected]>

Signed-off-by: Vibhu Jawa <[email protected]>

VibhuJawa · 2024-05-21T00:37:22Z

@ryantwolf , Thanks again for all the careful reviews. Appreciate the help . I think i should have those resolved.

ryantwolf

Looking good! Just a couple of nits and things I think you missed from my last review. I'll run some tests to do some final verifications too.

nemo_curator/utils/script_utils.py

setup.py

Signed-off-by: Vibhu Jawa <[email protected]>

ryantwolf

Good with me, thanks!

sarahyurick

LGTM, thanks! Added a couple questions for general discussion.

sarahyurick · 2024-05-21T21:09:48Z

nemo_curator/distributed_data_classification/generate_statistics.py

Just wanted to make sure there's nothing here we want to keep?

We can add it back as a followup if we think we need it .

sarahyurick · 2024-05-21T21:18:09Z

tutorials/distributed_data_classification/distributed_data_classification.ipynb

+    "input_file_path=\"/input_data_dir/\"\n",
+    "output_file_path = \"output_data_dir/\"\n",
+    "domain_model_path = \"domain_model.pth\"\n",
+    "quality_model_path = \"quality_model.pth\""


I see you opened #72, I think once we add support for that we should use real links, paths, etc. so that the user can run it without changing anything.

Should we open an issue for this?

Lets add to the issue for that to track we also change this file. Thanks for the suggestion. 👍🏼

VibhuJawa force-pushed the switch_to_crossfit branch from b41387e to 54b0a91 Compare May 9, 2024 18:32

VibhuJawa commented May 9, 2024

View reviewed changes

nemo_curator/distributed_data_classification/domain_classifier_inference.py Outdated Show resolved Hide resolved

sarahyurick reviewed May 9, 2024

View reviewed changes

sarahyurick reviewed May 10, 2024

View reviewed changes

examples/distributed_data_classification_examples/domain_api_example.py Outdated Show resolved Hide resolved

VibhuJawa commented May 16, 2024

View reviewed changes

VibhuJawa force-pushed the switch_to_crossfit branch from 368df8b to c8690d3 Compare May 16, 2024 23:23

VibhuJawa changed the base branch from main to pii-perf May 16, 2024 23:24

VibhuJawa changed the base branch from pii-perf to main May 16, 2024 23:24

VibhuJawa force-pushed the switch_to_crossfit branch from c8690d3 to bbee12f Compare May 16, 2024 23:28

VibhuJawa self-assigned this May 16, 2024

VibhuJawa force-pushed the switch_to_crossfit branch 2 times, most recently from 6c9d5ea to 6bff4fa Compare May 17, 2024 00:00

VibhuJawa changed the title ~~[WIP] Switch Models to use Crossfit~~ [REVIEW] Switch Models to use Crossfit May 17, 2024

VibhuJawa requested a review from ryantwolf May 17, 2024 00:02

ryantwolf requested changes May 17, 2024

View reviewed changes

VibhuJawa force-pushed the switch_to_crossfit branch from e2811e7 to cfd77f7 Compare May 20, 2024 19:01

sarahyurick reviewed May 20, 2024

View reviewed changes

examples/quality_classifier_example.py Outdated Show resolved Hide resolved

nemo_curator/distributed_data_classification/quality_classifier_multiple_models_inference.py Outdated Show resolved Hide resolved

VibhuJawa force-pushed the switch_to_crossfit branch 3 times, most recently from 540f5d8 to 982d7b6 Compare May 20, 2024 21:12

This was referenced May 20, 2024

[IMP] Hard code labels for the quality and domain classifier #71

Closed

[FEA] Make classifiers work with HuggingFace directly #72

Closed

ryantwolf reviewed May 20, 2024

View reviewed changes

VibhuJawa added 2 commits May 20, 2024 17:31

Working Domain Classifier with Crossfit

9f1c6fe

Signed-off-by: Vibhu Jawa <[email protected]>

Black on file

db6fc48

Signed-off-by: Vibhu Jawa <[email protected]>

VibhuJawa and others added 20 commits May 20, 2024 17:32

Address Reviews

d3e2fad

Signed-off-by: Vibhu Jawa <[email protected]>

Remove keep_prob_column

399712e

Signed-off-by: Vibhu Jawa <[email protected]>

Remove distributed_data_classification module

b1e2f9a

Signed-off-by: Vibhu Jawa <[email protected]>

Address Sarah's review about read_json

7d7d866

Signed-off-by: Vibhu Jawa <[email protected]>

Working Domain Classifier with Crossfit

8c46541

Signed-off-by: Vibhu Jawa <[email protected]>

Black on file

3856ea9

Signed-off-by: Vibhu Jawa <[email protected]>

style fixes

5cffb0b

Signed-off-by: Vibhu Jawa <[email protected]>

style fixes

1acf345

Signed-off-by: Vibhu Jawa <[email protected]>

First pass at switching to quality clasifier

0e42106

Signed-off-by: Vibhu Jawa <[email protected]>

Quality Classifier working

d010a24

Signed-off-by: Vibhu Jawa <[email protected]>

nb fix

a1b867d

Signed-off-by: Vibhu Jawa <[email protected]>

Make both classifiers work with labelling

9b1cd79

Signed-off-by: Vibhu Jawa <[email protected]>

Revert domain_api_example.py to main

97e89a0

Signed-off-by: Vibhu Jawa <[email protected]>

Added setup.py

c181b94

Signed-off-by: Vibhu Jawa <[email protected]>

Add crossfit to cpu install

24d046d

Signed-off-by: Vibhu Jawa <[email protected]>

Address Reviews

cc3088a

Signed-off-by: Vibhu Jawa <[email protected]>

Fix conflicts from rebase

63af368

Signed-off-by: Vibhu Jawa <[email protected]>

Update tutorials/distributed_data_classification/distributed_data_cla…

ecd95d4

…ssification.ipynb Co-authored-by: Ryan Wolf <[email protected]> Signed-off-by: Vibhu Jawa <[email protected]>

Update tutorials/distributed_data_classification/distributed_data_cla…

02fe2ef

…ssification.ipynb Co-authored-by: Ryan Wolf <[email protected]> Signed-off-by: Vibhu Jawa <[email protected]>

Update based on reviews

ca713c8

Signed-off-by: Vibhu Jawa <[email protected]>

VibhuJawa force-pushed the switch_to_crossfit branch from ca7b772 to ca713c8 Compare May 21, 2024 00:33

Update setup.py based on reviews

1f39839

Signed-off-by: Vibhu Jawa <[email protected]>

ryantwolf reviewed May 21, 2024

View reviewed changes

nemo_curator/utils/script_utils.py Show resolved Hide resolved

setup.py Outdated Show resolved Hide resolved

VibhuJawa added 2 commits May 21, 2024 09:25

Add -output-file-type

c6b5d9e

Signed-off-by: Vibhu Jawa <[email protected]>

Align Model Path instead of Model File Name

e1ec135

Signed-off-by: Vibhu Jawa <[email protected]>

ryantwolf self-requested a review May 21, 2024 19:59

ryantwolf approved these changes May 21, 2024

View reviewed changes

sarahyurick approved these changes May 21, 2024

View reviewed changes

VibhuJawa merged commit 9f8578b into NVIDIA:main May 21, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REVIEW] Switch Models to use Crossfit #58

[REVIEW] Switch Models to use Crossfit #58

VibhuJawa commented May 9, 2024 •

edited

Loading

VibhuJawa commented May 9, 2024

ayushdg commented May 9, 2024

VibhuJawa commented May 9, 2024

sarahyurick left a comment

sarahyurick left a comment

VibhuJawa May 16, 2024

ryantwolf May 17, 2024

ryantwolf May 17, 2024

VibhuJawa commented May 16, 2024

ryantwolf left a comment

ryantwolf May 17, 2024

ryantwolf May 17, 2024

VibhuJawa commented May 20, 2024

ryantwolf left a comment

VibhuJawa commented May 21, 2024

ryantwolf left a comment

ryantwolf left a comment

sarahyurick left a comment

sarahyurick May 21, 2024

VibhuJawa May 21, 2024

sarahyurick May 21, 2024

VibhuJawa May 21, 2024

[REVIEW] Switch Models to use Crossfit #58

[REVIEW] Switch Models to use Crossfit #58

Conversation

VibhuJawa commented May 9, 2024 • edited Loading

VibhuJawa commented May 9, 2024

ayushdg commented May 9, 2024

VibhuJawa commented May 9, 2024

sarahyurick left a comment

Choose a reason for hiding this comment

sarahyurick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VibhuJawa commented May 16, 2024

ryantwolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VibhuJawa commented May 20, 2024

ryantwolf left a comment

Choose a reason for hiding this comment

VibhuJawa commented May 21, 2024

ryantwolf left a comment

Choose a reason for hiding this comment

ryantwolf left a comment

Choose a reason for hiding this comment

sarahyurick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VibhuJawa commented May 9, 2024 •

edited

Loading