Skip to content

fix: don't crash the quantile token loss when no kernel is configured#9

Open
brettbj wants to merge 1 commit into
mainfrom
fix/quantile-loss-kernel-defaults
Open

fix: don't crash the quantile token loss when no kernel is configured#9
brettbj wants to merge 1 commit into
mainfrom
fix/quantile-loss-kernel-defaults

Conversation

@brettbj

@brettbj brettbj commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

How to recreate
Train with the shipped config/main.yaml (it sets custom_loss: true and a
quantile_token_loss block without a kernel key). The first training step
raises omegaconf.errors.ConfigAttributeError: Key 'kernel' is not in struct
from aggregate_first.

Why it's a bug
Loss.__init__ carefully guards kernel handling (if "kernel" in self.cfg.quantile_token_loss: ...), but both inner loss functions read self.cfg.quantile_token_loss.kernel.type unconditionally. Additionally, a kernel block with type but no factor left self.kernel_factor unset, raising AttributeError at first use. The default config shipped in this repo hits the first path.

How it was fixed
Resolve kernel_type (default "linear") and kernel_factor (default 1.0) once in __init__ via cfg.get(...) and reference only those attributes inside aggregate_first / loss_first. The linear kernel is the identity, so the defaults preserve previous intended behavior. Verified a forward pass of quantile_token_loss with the kernel-less config now returns a finite loss.

🤖 Generated with Claude Code

aggregate_first/loss_first read cfg.quantile_token_loss.kernel.type
unconditionally, so the shipped config/main.yaml (quantile_token_loss
without a kernel block) raised ConfigAttributeError on the first training
step; a kernel block without 'factor' likewise left self.kernel_factor
unset and raised AttributeError later.

Resolve kernel_type (default 'linear') and kernel_factor (default 1.0)
once in __init__ and reference only those attributes in the loss
functions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@brettbj

brettbj commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

if it hits this and kernel is not set, we might actually want it to crash rather than defaulting to linear without us knowing?

@burkh4rt

Copy link
Copy Markdown
Contributor

This was part of the reason I started working off the https://github.com/bbj-lab/cotorra/tree/coreopsis-stable branch

@burkh4rt burkh4rt requested a review from lukesolo-ml June 10, 2026 17:07
@burkh4rt burkh4rt closed this Jun 10, 2026
@burkh4rt burkh4rt deleted the fix/quantile-loss-kernel-defaults branch June 10, 2026 17:56
@burkh4rt burkh4rt restored the fix/quantile-loss-kernel-defaults branch June 10, 2026 17:57
@burkh4rt burkh4rt reopened this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants