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

Consolidate tgis finetuning #116

Merged
merged 12 commits into from
Aug 8, 2023

Conversation

gkumbhat
Copy link
Collaborator

@gkumbhat gkumbhat commented Aug 3, 2023

closes: #81

Changes

  • Remove fine-tuning module
  • Move .train function and related pre-processing function from fine-tuning modules to text-generation local module
  • Refactor save and load to provide portability between these
  • Add tests

Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
… concept

Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
…for evaluation

Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Copy link
Collaborator

@alex-jw-brooks alex-jw-brooks left a comment

Choose a reason for hiding this comment

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

Thanks Gaurav, great work! Some thoughts, but in general this looks a lot better

caikit_nlp/modules/text_generation/text_generation_tgis.py Outdated Show resolved Hide resolved
examples/utils.py Outdated Show resolved Hide resolved
if not model_name_override.endswith(loaded_base_model):
raise ValueError(
log.error(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this being made non fatal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because we are now re-training the models with fine-tuning, so model name may not be same as base_model name, os the above condition would fail.

examples/run_fine_tuning.py Outdated Show resolved Hide resolved
examples/run_fine_tuning.py Show resolved Hide resolved
try:
raw_model_text = model.run(datum.input).generated_text
except Exception as ex:
print(ex)
Copy link
Collaborator

Choose a reason for hiding this comment

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

oof. Any way to make this more narrow, or is it like a general exception? Or, should we at least catch separately for in vs not in TGIS?

gkumbhat and others added 4 commits August 7, 2023 16:49
Co-authored-by: Alex Brooks <alex.brooks@ibm.com>
Signed-off-by: Gaurav Kumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
Copy link
Collaborator

@alex-jw-brooks alex-jw-brooks left a comment

Choose a reason for hiding this comment

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

LGTM!

@alex-jw-brooks alex-jw-brooks merged commit 8ece6e3 into caikit:main Aug 8, 2023
4 checks passed
gkumbhat pushed a commit to gkumbhat/caikit-nlp that referenced this pull request Aug 24, 2023
Consolidate tgis finetuning
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
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.

Consolidate text-generation module with fine-tuning module
2 participants