Repository navigation
Cleaning Solver - #836
Draft
ndem0 wants to merge 1 commit into
Draft
Cleaning Solver#836ndem0 wants to merge 1 commit into
ndem0 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Solver-specific _compute_condition_loss overrides in SelfAdaptive/Competitive don’t apply the new _weight_condition_loss hook, causing functional inconsistency vs BaseSolver’s updated loss pipeline.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This PR refactors the solver subsystem to consolidate initialization in BaseSolver, simplify mixin responsibilities (notably multi-model vs ensemble forwarding), and separate additive regularization from multiplicative loss weighting via a new hook.
Changes:
- Merge solver component setup + weighting/loss initialization into
BaseSolver.__init__, updating core solver classes to pass all init args in one call. - Introduce
BaseSolver.forward(default: first model) and remove redundantforwardoverrides from SelfAdaptive/Competitive solvers. - Split loss post-processing into two hooks:
_regularize_condition_loss(additive) and_weight_condition_loss(multiplicative), migrating residual-based attention to the new weighting hook.
| File | Description |
|---|---|
| pina/_src/solver/base_solver.py | Expands __init__, adds default forward, and adds _weight_condition_loss hook into the loss pipeline. |
| pina/_src/solver/single_model_solver.py | Updates initialization to pass model/optimizer/scheduler/weighting/loss into BaseSolver.__init__. |
| pina/_src/solver/multi_model_solver.py | Updates initialization to pass models/optimizers/schedulers/weighting/loss into BaseSolver.__init__ and sets manual optimization directly. |
| pina/_src/solver/ensemble_solver.py | Updates initialization to pass models/optimizers/schedulers/weighting/loss into BaseSolver.__init__ and sets manual optimization directly. |
| pina/_src/solver/mixin/multi_model_mixin.py | Removes stacking forward; mixin now focuses on multi-optimizer configuration + convenience properties. |
| pina/_src/solver/mixin/manual_optimization_mixin.py | Removes _init_manual_optimization helper; callers now set automatic_optimization directly. |
| pina/_src/solver/mixin/residual_based_attention_mixin.py | Renames residual-attention hook to _weight_condition_loss and updates docstring accordingly. |
| pina/_src/solver/self_adaptive_physics_informed_solver.py | Removes redundant forward override following BaseSolver default-forward addition. |
| pina/_src/solver/competitive_physics_informed_solver.py | Removes redundant forward override following BaseSolver default-forward addition. |
| pina/_src/solver/REFACTORING.md | Adds design/architecture documentation for the refactor and new hook responsibilities. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| """ | ||
| return self.model(x) | ||
|
|
||
| def _compute_condition_loss(self, condition, data, batch_idx): |
| """ | ||
| return self.model(x) | ||
|
|
||
| def _compute_condition_loss(self, condition, data, batch_idx): |
ndem0
force-pushed
the
solver_improvement
branch
from
November 6, 2026 09:12
d56cf81 to
f2e5bab
Compare
Contributor
|
Code Coverage SummaryResults for commit: f2e5bab Minimum allowed coverage is ♻️ This comment has been updated with latest results |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes #ISSUE_NUMBER.
Checklist