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

Evaluate training set in the training loop (GBM) #2907

Merged
merged 16 commits into from
Jan 14, 2023
Merged

Conversation

jppgks
Copy link
Contributor

@jppgks jppgks commented Jan 10, 2023

Complement to #2856

ludwig/trainers/trainer_lightgbm.py Show resolved Hide resolved
ludwig/trainers/trainer_lightgbm.py Show resolved Hide resolved
ludwig/trainers/trainer_lightgbm.py Outdated Show resolved Hide resolved
ludwig/utils/gbm_utils.py Outdated Show resolved Hide resolved
ludwig/utils/gbm_utils.py Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jan 10, 2023

Unit Test Results

         6 files  ±0           6 suites  ±0   4h 46m 5s ⏱️ + 20m 49s
  3 886 tests ±0    3 812 ✔️ ±0    74 💤 ±0  0 ±0 
11 655 runs  ±0  11 433 ✔️ ±0  222 💤 ±0  0 ±0 

Results for commit 70de7d5. ± Comparison against base commit 76516b1.

♻️ This comment has been updated with latest results.

@arnavgarg1 arnavgarg1 self-requested a review January 10, 2023 22:47
@jeffkinnison jeffkinnison self-requested a review January 10, 2023 22:56
ludwig/trainers/trainer_lightgbm.py Show resolved Hide resolved
ludwig/trainers/trainer_lightgbm.py Outdated Show resolved Hide resolved
ludwig/utils/gbm_utils.py Show resolved Hide resolved
Copy link
Contributor

@jeffkinnison jeffkinnison left a comment

Choose a reason for hiding this comment

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

LGTM! Great work smoothing out some rough edges in addition to recording training set metrics.

Completely as a question for another day, would it be possible to remove train_step from LightGBMRayTrainer to reduce code duplication? Maybe by recording the various LGBM model classes within their trainer classes?

@jppgks jppgks requested a review from jeffkinnison January 12, 2023 15:19
Copy link
Collaborator

@tgaddair tgaddair left a comment

Choose a reason for hiding this comment

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

Nice!

@jppgks jppgks force-pushed the no-eval-train-gbm branch 2 times, most recently from 2151f24 to d856dc9 Compare January 13, 2023 20:43
@jppgks jppgks force-pushed the no-eval-train-gbm branch from 4ecc3f7 to 70de7d5 Compare January 14, 2023 17:11
@jppgks jppgks merged commit 3d0c204 into master Jan 14, 2023
@jppgks jppgks deleted the no-eval-train-gbm branch January 14, 2023 20:28
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.

4 participants