fix(build): add the base hooks the #1789 split slices override - #1991
Conversation
Every split slice of #1789 fails to compile against master with CS0115, and always on the same two members. The slices carry the overrides; the base declarations were only ever on the #1789 branch itself, so no slice can build until that whole PR lands. Both are added here so the slices compile against master in any order. LayerBase.Forward stops being abstract. It becomes the single point every forward call passes through, delegating to a new protected virtual ForwardTraced, and recording the call when a LayerForwardObserver is attached. That is what makes a model's real dataflow recoverable from one forward pass instead of from a declared topology that can drift. Layers that still override Forward keep working and are simply invisible to tracing. ForwardTraced's default throws NotSupportedException, not NotImplementedException: a layer that overrides neither has no computation at all, and will not acquire one later. FinancialModelBase gains a virtual TrainingOptimizer, and the shared tape path passes it to TrainWithTape. A model that built its own optimizer from its options previously had no way to hand it over, so the framework default silently won. Verified: net8.0 and net471 both build with 0 errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Deployment failed for project aidotnet-playground-api with the following error: Learn More: https://vercel.com/franklins-projects-02a0b5a0?upgradeToPro=build-rate-limit |
|
Warning Review limit reached
Next review available in: 5 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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. Comment |
|
Measured on
Every CS0115 ( |
Both conflicts are this branch meeting its own extract. #1991 lifted the two base hooks out of here so the split slices could build against master, master then reviewed them, and the review changed things this branch never saw. LayerBase.Forward/ForwardTraced: took master's copy, which throws NotSupportedException rather than NotImplementedException -- the type does not support the call and never will without an override, where NotImplementedException promises an implementation that is not coming. Kept this branch's MIGRATION paragraph instead of master's: it names ADNSHAPE004 as the thing that counts the un-migrated layers, and that diagnostic exists here and not on master, so master's wording is the one that goes stale after the merge. FinancialModelBase.TrainingOptimizer: identical member, master's doc. It records why the hook exists -- a model that builds its own optimizer from its options had no way to hand it to the shared TrainWithTape path, so the framework default silently won -- which is the part worth keeping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every split slice of #1789 fails to compile against master with CS0115, and always on the same two members. The slices carry the overrides; the base declarations were only ever on the #1789 branch itself, so no slice can build until that whole PR lands. Both are added here so the slices compile against master in any order. LayerBase.Forward stops being abstract. It becomes the single point every forward call passes through, delegating to a new protected virtual ForwardTraced, and recording the call when a LayerForwardObserver is attached. That is what makes a model's real dataflow recoverable from one forward pass instead of from a declared topology that can drift. Layers that still override Forward keep working and are simply invisible to tracing. ForwardTraced's default throws NotSupportedException, not NotImplementedException: a layer that overrides neither has no computation at all, and will not acquire one later. FinancialModelBase gains a virtual TrainingOptimizer, and the shared tape path passes it to TrainWithTape. A model that built its own optimizer from its options previously had no way to hand it over, so the framework default silently won. Verified: net8.0 and net471 both build with 0 errors. Co-authored-by: t <t@e.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Both conflicts are this branch meeting its own extract. #1991 lifted the two base hooks out of here so the split slices could build against master, master then reviewed them, and the review changed things this branch never saw. LayerBase.Forward/ForwardTraced: took master's copy, which throws NotSupportedException rather than NotImplementedException -- the type does not support the call and never will without an override, where NotImplementedException promises an implementation that is not coming. Kept this branch's MIGRATION paragraph instead of master's: it names ADNSHAPE004 as the thing that counts the un-migrated layers, and that diagnostic exists here and not on master, so master's wording is the one that goes stale after the merge. FinancialModelBase.TrainingOptimizer: identical member, master's doc. It records why the hook exists -- a model that builds its own optimizer from its options had no way to hand it to the shared TrainWithTape path, so the framework default silently won -- which is the part worth keeping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What
Adds the two base-class members that every
#1789split slice overrides but that only ever existed on the#1789branch itself:LayerBase<T>.ForwardTraced(Tensor<T>)src/NeuralNetworks/Layers/LayerBase.csprotected override Tensor<T> ForwardTraced(...)FinancialModelBase<T>.TrainingOptimizersrc/Finance/Base/FinancialModelBase.csFactorVAE<T>and siblings declareprotected override ... TrainingOptimizerPlus
src/NeuralNetworks/Graph/LayerForwardObserver.cs, whichForwardnow consults.Why standalone
This is the shared failure. Slices #1967–#1983 each target
masterindependently, so every one of them hits the sameCS0115: no suitable method found to overrideuntil #1789 lands in full. Landing just the base declarations unblocks all of them at once, in any merge order.The
ForwardchangeLayerBase<T>.Forwardgoes fromabstracttovirtual. It now delegates toForwardTracedand records the call when an observer is attached — the same__call__/forwardsplit PyTorch uses, so the framework has one place to stand between the caller and the computation. Existing layers that overrideForwardare unaffected; they keep working and are simply invisible to tracing.ForwardTraced's default throwsNotSupportedExceptionrather thanNotImplementedException: a layer overriding neither has no computation at all, and is not going to acquire one later.Verification
🤖 Generated with Claude Code