Logging Fixes & Enhancements #571
Conversation
- Added new field `log_level` to `TrainingArgs` for improved training configuration. - Updated `print_masked_samples` to use print statements instead of logger for better visibility during data processing. - Integrated logging level configuration into the training setup, enabling dynamic log level adjustment. - Fixed command construction so log level is propagated down into the training command Signed-off-by: Oleg S <[email protected]>
| setup_root_logger(train_args.log_level) | ||
| setup_metric_logger("async", None, train_args.ckpt_output_dir) | ||
|
|
||
| logger = logging.getLogger("instructlab.training") |
There was a problem hiding this comment.
could this be inferred from:
| logger = logging.getLogger("instructlab.training") | |
| logger = logging.getLogger(__name__) |
?
| "Pretraining" if unmask else "Instruction" + " ex sample %d: %s", | ||
| i + 1, | ||
| text, | ||
| print(f"\033[35mOriginal Input: {orig_text}\n\033[0m") |
There was a problem hiding this comment.
I read your issue (#569) for this reversion and I see your point.
A user might not always wish to see these outputs, and may want to silence them by setting the log level higher than the level that this is logged. Maybe an alternative solution would be logger.info rather than logger.debug so that we could still use the logger here?
Having the formatted outputs always logged to the user isn't the standard behavior for other libraries (namely, Axolotl), so the previous behavior, while useful to us during development, might not be desirable.
There was a problem hiding this comment.
@JamesKunstle I'm intentionally avoiding using the logger here because it applies formatting which misrepresents the actual content.
A user might not always wish to see these outputs, and may want to silence them by setting the log level higher than the level that this is logged. Maybe an alternative solution would be logger.info rather than logger.debug so that we could still use the logger here?
I would argue against this, for a few reasons:
- Most issues during training stem from data, therefore having a preview of what data you send to the model is a crucial step
- There is already a lot of noise which users do not care about, so providing an additional preview of their data does little to make that worse. If users don't wish to see a preview of their data, then all the other training logs that are printed today should also be muted. At this point, the program would likely be running somewhere where the user won't care or look at the logs anyway, so there's no reason to omit an otherwise useful step.
JamesKunstle
left a comment
JamesKunstle
left a comment
There was a problem hiding this comment.
Generally in favor of all changes- question about switching logger.debug to print.
3 participants
Addresses issue #569
Enhance training configuration and logging
log_leveltoTrainingArgsfor improved training configuration.print_masked_samplesto use print statements instead of logger for better visibility during data processing.