My First Pull Request to Someone Else's Repo Was a Rounding Error
A numerical-stability bug in LARQL's MEMIT path sat fixed in my fork for four months. Here's the bug, the fix, and why it took me so long to send it upstream.
The PR is submitted, not merged. If that changes I'll put a line up here. Link: https://github.com/chrishayuk/larql/pull/339
Four months of "later"
Back in April I fixed a bug. A real one. Took me a night. Then I left it sitting in a fork on my machine and moved on, because I had a pipeline to run and the pipeline worked now.
That was April 21st or 22nd. I'm honestly not sure which. It was late.
It sat there until August 29th.
What finally moved it wasn't guilt. It was a cleanup pass. I've been going through every repo I own with one rule: commit everything, give every commit a remote. No more work that only exists on one disk. Boring. Necessary. Somewhere in that sweep I hit the LARQL fork and found a diff that had never been committed. So I committed it (ca9a0699 in the fork, if you go looking) and then did the thing I should have done in April. I opened upstream's trace.rs and looked at line 303.
The f32 accumulator was still there.
Four months. 1,139 stars, 204 forks, a maintainer who merges PRs the same day they come in. And the bug I'd fixed on my desk was still in main because I never told anyone about it.
What the covariance is for
Quick version. LARQL has a MEMIT path. MEMIT edits facts directly into a model's weights. To do that without wrecking the layer it needs to know what the FFN activations normally look like there, so it estimates their covariance: C = (KแตK)/N over thousands of sampled token positions. Then it runs a Cholesky factorization on C and solves against it.
That's it. That's all the math this post needs. A big symmetric matrix, built by summing outer products, then factored.
The bug, in the words of the comment that caused it
Here's what was sitting above the accumulator in estimate_ffn_covariance:
"Float64 would be safer but Array2<f32> suffices at our scales."
Whoever wrote that was right. At their scales. I was not at their scales.
I was injecting facts into Gemma 4 26B-A4B on my RTX 5090. That model has ffn_dim = 2112 and activation magnitudes around 10ยณ. Square that and each entry of the accumulator is on the order of 10โถ, times N rows. f32 gives you about 7 significant digits. So after enough rows the small contributions stop landing. They fall off the bottom of the mantissa and they're just gone.
The matrix you end up with looks fine. Right shape. Big entries roughly right. But it's no longer numerically positive semi-definite, which is the one property Cholesky actually needs. Cholesky hits a negative pivot and MEMIT dies with "Cholesky failed".
No warning. No degraded result. The whole compile step falls over.
Why f64 after isn't f64
This is the part that took me a minute.
Upstream's memit.rs already converts the covariance to f64 before running Cholesky. There's a cov_f64. So on paper the precision problem was handled.
Except it wasn't. You can't convert your way back to precision you already threw away. The loss happened during accumulation, inside trace.rs, one += at a time. By the time memit.rs sees the matrix it's already wrong. Casting a wrong f32 to f64 just gives you the same wrong number with more zeros after it.
So the fix moves the f64 upstream of the loss. Accumulate KแตK in f64. Downcast to f32 once, at the boundary, when the function returns:
// Accumulate in f64. With ffn_dim in the thousands and activation
// magnitudes ~1e3 (observed on Gemma 4 26B-A4B, ffn_dim = 2112), each
// entry grows to ~1e6 * N; f32 (24-bit mantissa) loses the low-order
// contributions and the accumulated Gram matrix stops being numerically
// PSD, so the downstream Cholesky in memit.rs hits a negative pivot.
// Converting to f64 *after* accumulation (memit.rs `cov_f64`) cannot
// recover precision that was already lost here. We downcast to f32 once,
// at the boundary, when returning.
let mut ktk = Array2::<f64>::zeros((ffn_dim, ffn_dim));
// ...
for j in 0..ffn_dim {
ktk[[i, j]] += vi * (row[j] as f64);
}
Memory cost: 2112ยฒ ร 8 bytes is about 36 MB instead of 18. For one temporary matrix. Nobody will notice.
There's a second part in the PR. If Cholesky still fails, on some architecture nobody has profiled yet, memit.rs now escalates the ridge ร10, up to 1000ร, before giving up. It logs each escalation so whoever is running it can see it happen. That's defensive. It is not the fix. Part one is the fix. Part two is so the next person gets a log line instead of a crash.
One thing I left out on purpose. My April fix hard-coded ridge = 10.0 for Gemma 4. Ugly. Worked. Upstream has since made ridge an env var, LARQL_MEMIT_RIDGE, which is the right design, and they got there without me. So the config half was already solved. I only had to send the numerics half.
Porting a patch that no longer applies
The April diff didn't apply. Of course it didn't. The repo had moved 1,239 commits since my fork point, and seventeen of them touched that one file. One of the three files I'd changed didn't even exist anymore. It got split up and moved.
So on the 30th I forked properly, github.com/Ai-Eli-ML/larql, and ported the change by hand onto upstream main. Read the new file. Find where the accumulator lives now. Put the f64 in. Write the comment long enough that the next person doesn't "fix" it back to f32 to save memory. Build on the pinned 1.98.0 toolchain. Open the PR.
It was not hard. That's kind of the point. The port took less time than I spent, spread over four months, thinking "I should really send that upstream."
Why I needed this
I'll be honest about the motive. This wasn't charity.
I run a fact-injection pipeline locally: LARQL โ GGUF โ Ollama on an RTX 5090. It compiles facts into model weights. When Cholesky fails in that pipeline it isn't a warning I can scroll past. It's the compile step dying. I need this to work on big local models, so I fixed it for me first.
The PR is the give-it-back step. It's also the step I skipped for four months.
First PR. Submitted, not merged.
I want to be careful with the wording here because I've seen people get this wrong and it looks bad later. The PR is open. It is not merged. It is not accepted. It's here: https://github.com/chrishayuk/larql/pull/339
If the maintainer merges it, I'll add a line at the top of this post. If they want changes I'll make them. Might get closed. I'll say that too.
And the reputation thing, said plainly: I did not think of myself as someone who sends PRs. I build my own stuff. I fork things and bend them to what I need. Sending the fix back felt like something other people do. People with more standing. Or cleaner code. I don't know what the story was exactly.
That story cost me four months. The bug was real. The fix was small. Someone would've looked at it, the repo is busy enough. I was the only thing in the way.
What I'm doing differently
Two things.
Commit and push the day it works. Not when it's clean. Not after the cleanup pass. The day it works. If it only lives on one disk it didn't happen.
And check upstream before assuming they've fixed it. I spent four months half-assuming a project with that many people on it had surely caught something this basic. Line 303 said otherwise. Open the file. Look.
That's it. A rounding error and a PR I should have opened in April.
Fork: github.com/Ai-Eli-ML/larql. Upstream: chrishayuk/larql. PR: https://github.com/chrishayuk/larql/pull/339