fix: propagate LightGBM iteration failures - #2684
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey 小H (@xiaoh1024) 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
There was a problem hiding this comment.
Pull request overview
This PR fixes LightGBM training error-handling so that exceptions thrown during TrainUtils.updateOneIteration are logged and propagated (instead of being treated as “early termination” by setting state.isFinished = true). This aligns distributed training behavior with Spark’s normal task/job failure semantics and prevents returning partial models after worker/network failures.
Changes:
- Update
TrainUtils.updateOneIterationto log iteration failures and rethrow the original exception (leavingstate.isFinishedunchanged on failure). - Add a regression test that uses a failing
LightGBMBoosterto verify exception propagation and that training state is not marked as finished.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/TrainUtils.scala | Stop converting iteration exceptions into “finished” state; log and rethrow to let Spark handle failures. |
| lightgbm/src/test/scala/com/microsoft/azure/synapse/ml/lightgbm/split1/TrainUtilsSuite.scala | Add deterministic regression coverage ensuring iteration exceptions propagate and do not set state.isFinished. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Sorry, I am unable to sign the CLA at this time, so I am closing this pull request. Thank you for your time. |
## Summary Stop treating native LightGBM iteration exceptions as successful early completion. Log and rethrow the original failure so Spark fails the task instead of constructing a partial model, with a deterministic regression test that preserves the exception and unfinished state. ## Prompting Intent Investigate the value of closed PR microsoft#2684 and the linked distributed LightGBM issue family, then implement evidence-backed actionable work in an isolated worktree and validate it locally and on Microsoft Fabric. ## Linked Sources - Closed contribution: microsoft#2684 - Historical multiclass report: microsoft#569 - Historical binary socket report: microsoft#728 - Retry diagnostics fix: microsoft#2612 ## Rationale Normal LightGBM completion is already returned by the native update call. An exception represents failure, not early stopping. Rethrowing the same object preserves the native cause and lets Spark apply task and job failure semantics; swallowing it can return an incomplete model that appears valid. This deliberately does not claim to repair every network or executor failure in the broader issue family. Co-authored-by: Copilot <[email protected]>
## Summary Stop treating native LightGBM iteration exceptions as successful early completion. Log and rethrow the original failure so Spark fails the task instead of constructing a partial model, with a deterministic regression test that preserves the exception and unfinished state. ## Prompting Itent Investigate the value of closed PR #2684 and the linked distributed LightGBM issue family, then implement evidence-backed actionable work in an isolated worktree and validate it locally and on Microsoft Fabric. ## Linked Sources - Closed contribution: #2684 - Historical multiclass report: #569 - Historical binary socket report: #728 - Retry diagnostics fix: #2612 ## Rationale Normal LightGBM completion is already returned by the native update call. An exception represents failure, not early stopping. Rethrowing the same object preserves the native cause and lets Spark apply task and job failure semantics; swallowing it can return an incomplete model that appears valid. This deliberately does not claim to repair every network or executor failure in the broader issue family.
|
Thanks for flagging this 小H (@xiaoh1024) I've made a follow up PR and merged the fixed: #2695 |
Related Issues/PRs
Related to #569 and #728.
This PR addresses the error-handling behavior described in those issues. It does
not attempt to resolve the underlying causes of worker or network failures.
What changes are proposed in this pull request?
TrainUtils.updateOneIterationcurrently catches exceptions raised during aLightGBM iteration and sets
state.isFinished = true.This makes an iteration failure indistinguishable from legitimate training
completion. In distributed training, if one native worker fails, peer workers
can receive errors such as:
SynapseML then logs the exception as early termination. As a result, the fit can
return a partial model instead of reporting the underlying task failure.
This change:
state.isFinishedunchanged when an iteration throws;It also adds a deterministic regression test using a failing booster. The test
verifies that the original exception is propagated and the training state is not
marked as finished.
How is this patch tested?
The focused
TrainUtilsSuitepasses, including the new regression test:Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?