Skip to content

chore: logging cleanup#3482

Merged
winglian merged 1 commit intomainfrom
chore/logger-cleanup
Mar 16, 2026
Merged

chore: logging cleanup#3482
winglian merged 1 commit intomainfrom
chore/logger-cleanup

Conversation

@NanoCode012
Copy link
Collaborator

@NanoCode012 NanoCode012 commented Mar 9, 2026

Description

Clean redundant params passed.

Motivation and Context

How has this been tested?

AI Usage Disclaimer

Screenshots (if appropriate)

Types of changes

Social Handles (Optional)

Summary by CodeRabbit

  • Chores
    • Updated logging behavior across distributed training components to emit messages from all processes rather than restricting to primary processes. This increases logging visibility during training but may result in more verbose output in multi-process scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e1e0e1d3-61cf-4305-818d-045853d2d4fc

📥 Commits

Reviewing files that changed from the base of the PR and between 43b1c80 and a4a41c4.

📒 Files selected for processing (9)
  • src/axolotl/cli/merge_sharded_fsdp_weights.py
  • src/axolotl/core/trainers/mixins/checkpoints.py
  • src/axolotl/monkeypatch/attention/flex_attn.py
  • src/axolotl/monkeypatch/ring_attn/patch.py
  • src/axolotl/monkeypatch/tiled_mlp/patch.py
  • src/axolotl/train.py
  • src/axolotl/utils/callbacks/dynamic_checkpoint.py
  • src/axolotl/utils/data/shared.py
  • src/axolotl/utils/schemas/validation.py
💤 Files with no reviewable changes (8)
  • src/axolotl/utils/schemas/validation.py
  • src/axolotl/core/trainers/mixins/checkpoints.py
  • src/axolotl/utils/data/shared.py
  • src/axolotl/monkeypatch/ring_attn/patch.py
  • src/axolotl/monkeypatch/tiled_mlp/patch.py
  • src/axolotl/cli/merge_sharded_fsdp_weights.py
  • src/axolotl/utils/callbacks/dynamic_checkpoint.py
  • src/axolotl/train.py

📝 Walkthrough

Walkthrough

This PR systematically removes the main_process_only=True argument from eight LOG calls across the codebase, allowing logging messages to be emitted from all processes rather than exclusively from the main process. No functional logic changes are introduced; only logging behavior is modified to increase cross-process visibility.

Changes

Cohort / File(s) Summary
Training and checkpoint logging
src/axolotl/cli/merge_sharded_fsdp_weights.py, src/axolotl/core/trainers/mixins/checkpoints.py, src/axolotl/train.py, src/axolotl/utils/callbacks/dynamic_checkpoint.py
Removed main_process_only=True from LOG calls in model setup, optimizer/scheduler saving, and dynamic checkpoint operations, allowing these messages to emit from all processes.
Attention and computation patch logging
src/axolotl/monkeypatch/attention/flex_attn.py, src/axolotl/monkeypatch/ring_attn/patch.py, src/axolotl/monkeypatch/tiled_mlp/patch.py
Removed main_process_only=True from LOG calls in flex attention compilation and ring/tiled MLP patching routines, broadening logging visibility across processes.
Data utility and validation logging
src/axolotl/utils/data/shared.py, src/axolotl/utils/schemas/validation.py
Removed main_process_only=True from LOG calls in dataset preprocessing and evaluation packing validation, enabling logging on all processes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • winglian
  • djsaunde
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: logging cleanup' accurately reflects the main change across all files: removing main_process_only=True arguments from logging calls throughout the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/logger-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

📖 Documentation Preview: https://69aeb876ed377264c7011b61--resonant-treacle-0fd729.netlify.app

Deployed on Netlify from commit a4a41c4

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/axolotl/monkeypatch/attention/flex_attn.py 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@winglian winglian merged commit d8a646c into main Mar 16, 2026
19 of 20 checks passed
@winglian winglian deleted the chore/logger-cleanup branch March 16, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants