feat(runtime): Chronos-Bolt as a forecast candidate, in a pinned image, checked within a tolerance #667

Merged
admin merged 5 commits from feat/forecast-chronos into dev 2026-10-23 09:19:24 +00:00
Owner

Part of #665. Builds on the runner boundary from #666.

forecast can now try the pretrained Chronos-Bolt models as candidates: chronos-tiny, chronos-mini, chronos-small and chronos-base. They are backtested on the same windows, scored the same way, and earn a place in the median only by scoring among the best kinds. They are opt-in, and all still means only the Go kinds.

Design, as agreed on #665

  • Pinned image. sisyphus-chronos:<hash> is built on the worker from a Dockerfile generated in chronos.go. Its pieces are pinned:
    • the base is python:3.12-slim, pinned by digest;
    • 42 packages are installed with pip --require-hashes, at the same versions on amd64 and arm64, with torch==2.8.0+cpu;
    • each variant's config.json and model.safetensors are downloaded at a pinned Hugging Face revision and checked against their SHA-256 before the image is kept.
  • Routing. The tag is a hash of the Dockerfile and the container script. The same hash is the label forecast-chronos:<hash>: a task with a Chronos candidate Needs it, and a --containers node Offers it. Only a worker with the same image takes the task.
  • Runner. All of a task's Chronos fits go to one container. It runs one thread, in batches of 64 windows, with --network none, 1 CPU and up to 4 GB of memory. It reads /input/data and writes /output/answers.json. Answers of the wrong shape fail the task.
  • Verification. forecast now Varies and Compares:
    • outputs with no Chronos candidate must be the same bytes, as before;
    • with a Chronos candidate, every Go candidate must still match exactly;
    • each Chronos candidate's forecast, scores and residuals must be within 1e-3 of the largest |forecast|.
    • On 102 M4 series I measured the spread: the same settings give the same bits, threads 1 vs 4 differ by up to 1e-6, and batch 64 vs 1 by up to 1e-4. The task fixes threads and batch size. The aarch64 spread is not measured yet.
    • decide still runs once, in Aggregate.
  • forecasts refuses Chronos kinds for now. Per-candidate scores in the series output, with Compare re-deriving the choice, are PR2b.

Evidence

  • I built the image locally from the generated Dockerfile: 281 s and 3.93 GB. The pip hash checks and model sha256 checks passed.
  • TestChronosForecastsARealSeriesInItsImage (realimages) runs the real image on a series, gets the same bytes twice, and Compare agrees. It passed locally in 24 s.
  • Unit tests use a pretend engine and cover:
    • scoring like any other candidate;
    • Needs and Offers;
    • refusing Chronos without containers;
    • answers of the wrong shape;
    • Compare: exact for Go candidates, inside and outside the tolerance, symmetric.
  • Runtime coverage is 100% when combined with realimages. The only line the ordinary suite leaves uncovered is the docker engine default, which the real-image test covers.

Docs

  • docs/forecast-benchmark.md has a new Chronos-Bolt section: the M4 results from #579, how the image is pinned, the confinement, and the tolerance with its measured numbers.
  • The user guide mentions the opt-in choice.
  • The security model and the verification section of the README still say that no workload compares, or that only qchem does after #661. I'll add forecast there when I rebase onto #661, to avoid conflicting edits on the same lines.

CI cost, a question for you, Lior

The realimages job will now build this image: about 4 GB, including 1.13 GB for all four variants' weights. That will sit on the runners' 24 GB Docker tmpfs from #314. If that's too heavy, I can split the image per variant and test with tiny only, which is about 3 GB less. Your call.

Part of #665. Builds on the runner boundary from #666. `forecast` can now try the pretrained Chronos-Bolt models as candidates: `chronos-tiny`, `chronos-mini`, `chronos-small` and `chronos-base`. They are backtested on the same windows, scored the same way, and earn a place in the median only by scoring among the best kinds. They are opt-in, and `all` still means only the Go kinds. ### Design, as agreed on #665 - **Pinned image.** `sisyphus-chronos:<hash>` is built on the worker from a Dockerfile generated in `chronos.go`. Its pieces are pinned: - the base is `python:3.12-slim`, pinned by digest; - 42 packages are installed with `pip --require-hashes`, at the same versions on amd64 and arm64, with `torch==2.8.0+cpu`; - each variant's `config.json` and `model.safetensors` are downloaded at a pinned Hugging Face revision and checked against their SHA-256 before the image is kept. - **Routing.** The tag is a hash of the Dockerfile and the container script. The same hash is the label `forecast-chronos:<hash>`: a task with a Chronos candidate `Needs` it, and a `--containers` node `Offers` it. Only a worker with the same image takes the task. - **Runner.** All of a task's Chronos fits go to one container. It runs one thread, in batches of 64 windows, with `--network none`, 1 CPU and up to 4 GB of memory. It reads `/input/data` and writes `/output/answers.json`. Answers of the wrong shape fail the task. - **Verification.** `forecast` now `Varies` and `Compare`s: - outputs with no Chronos candidate must be the same bytes, as before; - with a Chronos candidate, every Go candidate must still match exactly; - each Chronos candidate's forecast, scores and residuals must be within **1e-3** of the largest |forecast|. - On 102 M4 series I measured the spread: the same settings give the same bits, threads 1 vs 4 differ by up to 1e-6, and batch 64 vs 1 by up to 1e-4. The task fixes threads and batch size. The aarch64 spread is not measured yet. - `decide` still runs once, in `Aggregate`. - **`forecasts` refuses Chronos kinds** for now. Per-candidate scores in the series output, with Compare re-deriving the choice, are PR2b. ### Evidence - I built the image locally from the generated Dockerfile: 281 s and 3.93 GB. The pip hash checks and model sha256 checks passed. - `TestChronosForecastsARealSeriesInItsImage` (realimages) runs the real image on a series, gets the same bytes twice, and `Compare` agrees. It passed locally in 24 s. - Unit tests use a pretend engine and cover: - scoring like any other candidate; - Needs and Offers; - refusing Chronos without containers; - answers of the wrong shape; - Compare: exact for Go candidates, inside and outside the tolerance, symmetric. - Runtime coverage is 100% when combined with realimages. The only line the ordinary suite leaves uncovered is the `docker` engine default, which the real-image test covers. ### Docs - `docs/forecast-benchmark.md` has a new Chronos-Bolt section: the M4 results from #579, how the image is pinned, the confinement, and the tolerance with its measured numbers. - The user guide mentions the opt-in choice. - The security model and the verification section of the README still say that no workload compares, or that only `qchem` does after #661. I'll add `forecast` there when I rebase onto #661, to avoid conflicting edits on the same lines. ### CI cost, a question for you, Lior The realimages job will now build this image: about 4 GB, including 1.13 GB for all four variants' weights. That will sit on the runners' 24 GB Docker tmpfs from #314. If that's too heavy, I can split the image per variant and test with `tiny` only, which is about 3 GB less. Your call.
Owner

On the CI image size: I would keep the single pinned image for now. At about 4 GB it fits comfortably within the runners’ 24 GB Docker tmpfs, and the real-image job exercises the actual pinned model variants together. Splitting into per-variant images adds tag/build paths and test complexity before we have evidence of runner pressure. If CI shows disk or memory contention, we can split then while retaining a lightweight integrity check for every variant.

/cc @rvnminers-A-and-N

On the CI image size: I would keep the single pinned image for now. At about 4 GB it fits comfortably within the runners’ 24 GB Docker tmpfs, and the real-image job exercises the actual pinned model variants together. Splitting into per-variant images adds tag/build paths and test complexity before we have evidence of runner pressure. If CI shows disk or memory contention, we can split then while retaining a lightweight integrity check for every variant. /cc @rvnminers-A-and-N
Author
Owner

Thanks, Lior. I'll keep the single pinned image.

The first realimages run on 3a5b539 built the image. Then TestChronosForecastsARealSeriesInItsImage failed after 331 s:

chronos: the command exited with status 125: Run 'docker run --help' for more information

Status 125 means Docker refused the container itself. The last line of stderr was only Docker's hint to read its help, so the error that mattered was lost. c2137bd keeps the line before that hint. It applies to every container workload and has a test.

My guess at the cause: Chronos is the only workload whose task asks for limits, --memory 4096m --cpus 1. If the runners' inner dockerd can't set a CPU or memory limit, for example because the cgroup controllers aren't delegated to it, it would refuse with exactly this status. The same run passed locally against a host dockerd.

Can you check whether docker run --rm --cpus 1 --memory 64m alpine:3.20 true works inside a runner? If it doesn't, I'd rather fix the runners than drop the limits, because a worker's owner can set the same limits with --containers-memory-mb/--containers-cpus. Either way, the next run on c2137bd should name the reason.

Thanks, Lior. I'll keep the single pinned image. The first realimages run on `3a5b539` built the image. Then `TestChronosForecastsARealSeriesInItsImage` failed after 331 s: ``` chronos: the command exited with status 125: Run 'docker run --help' for more information ``` Status 125 means Docker refused the container itself. The last line of stderr was only Docker's hint to read its help, so the error that mattered was lost. `c2137bd` keeps the line before that hint. It applies to every container workload and has a test. My guess at the cause: Chronos is the only workload whose task asks for limits, `--memory 4096m --cpus 1`. If the runners' inner dockerd can't set a CPU or memory limit, for example because the cgroup controllers aren't delegated to it, it would refuse with exactly this status. The same run passed locally against a host dockerd. Can you check whether `docker run --rm --cpus 1 --memory 64m alpine:3.20 true` works inside a runner? If it doesn't, I'd rather fix the runners than drop the limits, because a worker's owner can set the same limits with `--containers-memory-mb`/`--containers-cpus`. Either way, the next run on `c2137bd` should name the reason.
Owner

I do not have interactive shell access to the self-hosted runner host from this checkout, so I cannot run that one-off command directly on the existing machine. The fresh c2137bd CI run is exercising the same realimages runner, and the improved Docker error should tell us whether the 125 is specifically about the resource limits. If it still does not identify the cgroup capability, the safest next step is a tiny preflight docker run --rm --cpus 1 --memory 64m alpine:3.20 true in the existing realimages job, so it runs on that exact host; I would not remove the workload limits without that evidence.

/cc @rvnminers-A-and-N

I do not have interactive shell access to the self-hosted runner host from this checkout, so I cannot run that one-off command directly on the existing machine. The fresh `c2137bd` CI run is exercising the same realimages runner, and the improved Docker error should tell us whether the 125 is specifically about the resource limits. If it still does not identify the cgroup capability, the safest next step is a tiny preflight `docker run --rm --cpus 1 --memory 64m alpine:3.20 true` in the existing realimages job, so it runs on that exact host; I would not remove the workload limits without that evidence. /cc @rvnminers-A-and-N
Owner

The fresh c2137bd run still fails TestChronosForecastsARealSeriesInItsImage after 329 s with Docker exit 125, but the improved error is empty: no Docker error line was emitted before the generic help hint. So this does not confirm the cgroup hypothesis. I still cannot shell into the self-hosted host from this checkout. Could you add the exact docker run --rm --cpus 1 --memory 64m alpine:3.20 true probe as a preflight in the existing realimages job (or let me know an approved way to access that runner)? That will distinguish resource-limit support from a Chronos-specific path without dropping the safeguards.

/cc @rvnminers-A-and-N

The fresh `c2137bd` run still fails `TestChronosForecastsARealSeriesInItsImage` after 329 s with Docker exit 125, but the improved error is empty: no Docker error line was emitted before the generic help hint. So this does not confirm the cgroup hypothesis. I still cannot shell into the self-hosted host from this checkout. Could you add the exact `docker run --rm --cpus 1 --memory 64m alpine:3.20 true` probe as a preflight in the existing realimages job (or let me know an approved way to access that runner)? That will distinguish resource-limit support from a Chronos-specific path without dropping the safeguards. /cc @rvnminers-A-and-N
Owner

I added the exact Docker resource-limit probe as a preflight in the existing realimages job, before the long Chronos image test. It runs against the same runner and will tell us directly whether Docker supports --cpus/--memory there; actionlint and git diff --check passed locally. The change is on your branch as 0ed6455. Please squash-merge it along with your PR once its checks finish, if you are happy with the diagnostic.\n\n/cc @rvnminers-A-and-N

I added the exact Docker resource-limit probe as a preflight in the existing realimages job, before the long Chronos image test. It runs against the same runner and will tell us directly whether Docker supports --cpus/--memory there; actionlint and git diff --check passed locally. The change is on your branch as 0ed6455. Please squash-merge it along with your PR once its checks finish, if you are happy with the diagnostic.\n\n/cc @rvnminers-A-and-N
Author
Owner

Thanks, Lior. Your probe is kept as your own commit (1554688), so it will be credited to you when this is squashed. I rebased the branch onto 8839b6a for the refreshed base you asked for on #661.

Why the error was empty: Docker's CLI writes its error, then a blank line, then the hint to read its help. c2137bd skipped the hint but kept the blank line. 060f506 skips blank lines too, and its test now uses Docker's real layout. The rebase also brings in the docs (a3d00b5): the security model and the README now say that forecast with a Chronos-Bolt candidate is checked within a tolerance.

The next realimages run should show both your probe's result and Docker's own error line, if Chronos still fails.

Thanks, Lior. Your probe is kept as your own commit (`1554688`), so it will be credited to you when this is squashed. I rebased the branch onto `8839b6a` for the refreshed base you asked for on #661. Why the error was empty: Docker's CLI writes its error, then a blank line, then the hint to read its help. `c2137bd` skipped the hint but kept the blank line. `060f506` skips blank lines too, and its test now uses Docker's real layout. The rebase also brings in the docs (`a3d00b5`): the security model and the README now say that `forecast` with a Chronos-Bolt candidate is checked within a tolerance. The next realimages run should show both your probe's result and Docker's own error line, if Chronos still fails.
Author
Owner

Your probe found it. On 1554688, "Check Docker resource limits on this runner" fails before any Chronos code runs:

docker: Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: unable to apply cgroup configuration: cannot enter cgroupv2 "/sys/fs/cgroup/docker" with domain controllers -- it is in an invalid state

So the runner's inner dockerd can't put a container in a cgroup with limits. This isn't specific to Chronos: any task that asks for memory_mb/cpus, or any worker started with --containers-memory-mb/--containers-cpus, would fail the same way on those runners. Chronos is just the first real-image test to ask for limits. On my host's Docker (cgroup v2, systemd driver) the same command works.

Under cgroup v2 the root cgroup can't hold processes and also delegate controllers ("no internal processes"). A nested dockerd has to move its own processes into a child cgroup, then enable the controllers for the subtree, before it creates container cgroups. The docker:dind image's entrypoint does exactly that when it starts:

if [ -f /sys/fs/cgroup/cgroup.controllers ]; then
  mkdir -p /sys/fs/cgroup/init
  xargs -rn1 < /sys/fs/cgroup/cgroup.procs > /sys/fs/cgroup/init/cgroup.procs || :
  sed -e 's/ / +/g' -e 's/^/+/' < /sys/fs/cgroup/cgroup.controllers > /sys/fs/cgroup/cgroup.subtree_control
fi

If the runner containers start dockerd some other way, which may have changed with the #314 storage move, running that step first should fix it. The runners are yours, so I'll leave the change to you. Once the probe passes, the Chronos real-image test should get past status 125. I'd keep your probe in CI, because it turns a misconfigured runner into a clear one-line failure.

Your probe found it. On `1554688`, "Check Docker resource limits on this runner" fails before any Chronos code runs: ``` docker: Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: unable to apply cgroup configuration: cannot enter cgroupv2 "/sys/fs/cgroup/docker" with domain controllers -- it is in an invalid state ``` So the runner's inner dockerd can't put a container in a cgroup with limits. This isn't specific to Chronos: any task that asks for `memory_mb`/`cpus`, or any worker started with `--containers-memory-mb`/`--containers-cpus`, would fail the same way on those runners. Chronos is just the first real-image test to ask for limits. On my host's Docker (cgroup v2, systemd driver) the same command works. Under cgroup v2 the root cgroup can't hold processes and also delegate controllers ("no internal processes"). A nested dockerd has to move its own processes into a child cgroup, then enable the controllers for the subtree, before it creates container cgroups. The `docker:dind` image's entrypoint does exactly that when it starts: ```sh if [ -f /sys/fs/cgroup/cgroup.controllers ]; then mkdir -p /sys/fs/cgroup/init xargs -rn1 < /sys/fs/cgroup/cgroup.procs > /sys/fs/cgroup/init/cgroup.procs || : sed -e 's/ / +/g' -e 's/^/+/' < /sys/fs/cgroup/cgroup.controllers > /sys/fs/cgroup/cgroup.subtree_control fi ``` If the runner containers start dockerd some other way, which may have changed with the #314 storage move, running that step first should fix it. The runners are yours, so I'll leave the change to you. Once the probe passes, the Chronos real-image test should get past status 125. I'd keep your probe in CI, because it turns a misconfigured runner into a clear one-line failure.
Owner

The preflight has now failed on the self-hosted realimages runner before the Chronos test starts. That confirms the failure is in the constrained Docker launch path (and makes runner/cgroup configuration the likely area), rather than Chronos itself. GitHub has not exposed the job log until the full workflow finishes, so I do not yet have Docker’s exact stderr. Once the log is available I will share the precise message; in the meantime, the resource-limits hypothesis is supported.\n\n/cc @rvnminers-A-and-N

The preflight has now failed on the self-hosted realimages runner before the Chronos test starts. That confirms the failure is in the constrained Docker launch path (and makes runner/cgroup configuration the likely area), rather than Chronos itself. GitHub has not exposed the job log until the full workflow finishes, so I do not yet have Docker’s exact stderr. Once the log is available I will share the precise message; in the meantime, the resource-limits hypothesis is supported.\n\n/cc @rvnminers-A-and-N
Owner

The job log is available now; the preflight confirms the runner issue precisely. Docker reports: unable to apply cgroup configuration: cannot enter cgroupv2 "/sys/fs/cgroup/docker" with domain controllers -- it is in an invalid state (runner sisyphus-ci-4). So the Chronos test is not the cause: this runner’s Docker cgroup-v2 setup is invalid for launching even a minimal container with limits. This needs a runner/Docker host configuration fix; I have not changed or removed the workload limits.\n\n/cc @rvnminers-A-and-N

The job log is available now; the preflight confirms the runner issue precisely. Docker reports: `unable to apply cgroup configuration: cannot enter cgroupv2 "/sys/fs/cgroup/docker" with domain controllers -- it is in an invalid state` (runner `sisyphus-ci-4`). So the Chronos test is not the cause: this runner’s Docker cgroup-v2 setup is invalid for launching even a minimal container with limits. This needs a runner/Docker host configuration fix; I have not changed or removed the workload limits.\n\n/cc @rvnminers-A-and-N
Author
Owner

Thanks, Lior. That matches: runner sisyphus-ci-4. You restarted the runners at 04:41Z with the tmpfs and the 36 GB limit (#314). If the restart went through a different entrypoint, or put the runner's own processes in its root cgroup, that would explain why the nesting step is missing now. The fix is the init cgroup move plus cgroup.subtree_control, run before dockerd starts, as in the snippet above. I'll leave #667 as it is until the runners are fixed. Its only remaining failure is that probe, and I'll rerun the realimages job as soon as you say the runners are updated.

Thanks, Lior. That matches: runner `sisyphus-ci-4`. You restarted the runners at 04:41Z with the tmpfs and the 36 GB limit (#314). If the restart went through a different entrypoint, or put the runner's own processes in its root cgroup, that would explain why the nesting step is missing now. The fix is the `init` cgroup move plus `cgroup.subtree_control`, run before dockerd starts, as in the snippet above. I'll leave #667 as it is until the runners are fixed. Its only remaining failure is that probe, and I'll rerun the realimages job as soon as you say the runners are updated.
Owner

The runners are updated, so the realimages job can be rerun.

The startup script now does the init cgroup move and enables cgroup.subtree_control before dockerd starts, as in your snippet. Before it went live I checked it on the runner image itself: docker run --memory 64m --cpus 1 alpine reproduced your exact error, and with the fix it starts. Plain containers and docker network create still work.

This was never a #314 regression. The script never had this step: it has started dockerd the same way since the runners were set up on Oct 15. Nothing until Chronos asked for limits, so nothing showed it. Your probe is what caught it.

sisyphus-ci-2, -3 and -4 are on the fixed image and idle now. sisyphus-ci-1 is still finishing this PR's test job on the old image and switches over when that job ends, so the rerun will land on a fixed runner.

The runners are updated, so the realimages job can be rerun. The startup script now does the `init` cgroup move and enables `cgroup.subtree_control` before dockerd starts, as in your snippet. Before it went live I checked it on the runner image itself: `docker run --memory 64m --cpus 1 alpine` reproduced your exact error, and with the fix it starts. Plain containers and `docker network create` still work. This was never a #314 regression. The script never had this step: it has started dockerd the same way since the runners were set up on Oct 15. Nothing until Chronos asked for limits, so nothing showed it. Your probe is what caught it. `sisyphus-ci-2`, `-3` and `-4` are on the fixed image and idle now. `sisyphus-ci-1` is still finishing this PR's `test` job on the old image and switches over when that job ends, so the rerun will land on a fixed runner.
Author
Owner

Thanks, Lior, and thanks for testing it on the runner image first. Dev moved to f8e3f92 with #637, so I've rebased again (aa069f9; your probe is still your own commit). The fresh run gives the realimages job the updated runners from the start: the probe should pass now, and then the Chronos test runs for real.

Thanks, Lior, and thanks for testing it on the runner image first. Dev moved to `f8e3f92` with #637, so I've rebased again (`aa069f9`; your probe is still your own commit). The fresh run gives the realimages job the updated runners from the start: the probe should pass now, and then the Chronos test runs for real.
Owner

The fresh realimages job on aa069f9 passed end to end: the Docker resource-limit preflight and the Chronos real-image test both succeeded. The remaining CI checks are still running, so I am not treating the PR as merge-ready yet.\n\n/cc @rvnminers-A-and-N

The fresh realimages job on aa069f9 passed end to end: the Docker resource-limit preflight and the Chronos real-image test both succeeded. The remaining CI checks are still running, so I am not treating the PR as merge-ready yet.\n\n/cc @rvnminers-A-and-N
Author
Owner

Thanks, Lior. All 34 check-runs on aa069f9 are green now, including realimages and desktop, and the base is current dev. If you'd like to review the Chronos code before it lands, say so and I'll hold. Otherwise I'll squash-merge on my next pass, with your probe credited as co-author.

Thanks, Lior. All 34 check-runs on `aa069f9` are green now, including realimages and desktop, and the base is current `dev`. If you'd like to review the Chronos code before it lands, say so and I'll hold. Otherwise I'll squash-merge on my next pass, with your probe credited as co-author.
@@ -0,0 +310,4 @@
if !f.Containers {
return nil
}
return []string{ChronosLabel}
Owner

When --containers-memory-mb is set below 4096, this still advertises ChronosLabel and lists all variants. Container.Execute clamps Chronos’ 4096 MB request to the owner’s lower ceiling, so a selected candidate can OOM; chronosRunner.fit then fails the whole forecast task instead of leaving the Go candidates usable. Could Offers/modelChoices (or per-variant resource requirements) avoid advertising variants the owner cap cannot run? Please add a test for a sub-4-GB Confinement. This looks like a mismatch between the advertised capability and enforced limits, so I recommend holding merge until we settle the behavior.\n\n/cc @rvnminers-A-and-N

When --containers-memory-mb is set below 4096, this still advertises ChronosLabel and lists all variants. Container.Execute clamps Chronos’ 4096 MB request to the owner’s lower ceiling, so a selected candidate can OOM; chronosRunner.fit then fails the whole forecast task instead of leaving the Go candidates usable. Could Offers/modelChoices (or per-variant resource requirements) avoid advertising variants the owner cap cannot run? Please add a test for a sub-4-GB Confinement. This looks like a mismatch between the advertised capability and enforced limits, so I recommend holding merge until we settle the behavior.\n\n/cc @rvnminers-A-and-N
Owner

I reviewed the current diff. The realimages fix and tolerance tests look good, but I found one capability/resource edge case and left an inline note on Chronos Offers: a worker with an owner memory ceiling below 4096 MB still advertises the generic Chronos label and all variants, while execution clamps the request to that lower ceiling and can fail the whole forecast task. Please hold the squash until we settle that behavior and add coverage; I do not think this is ready to land unchanged.\n\n/cc @rvnminers-A-and-N

I reviewed the current diff. The realimages fix and tolerance tests look good, but I found one capability/resource edge case and left an inline note on Chronos Offers: a worker with an owner memory ceiling below 4096 MB still advertises the generic Chronos label and all variants, while execution clamps the request to that lower ceiling and can fail the whole forecast task. Please hold the squash until we settle that behavior and add coverage; I do not think this is ready to land unchanged.\n\n/cc @rvnminers-A-and-N
@@ -0,0 +397,4 @@
for _, v := range slices.Concat(p.Forecast, q.Forecast) {
scale = max(scale, math.Abs(v))
}
limit := chronosTolerance * max(scale, 1e-12)
Owner

This scale also governs MAE/RMSE, step errors and backtest residuals, not just forecast values. For a valid series whose forecast is near zero but whose values/residuals are large (for example, a strongly alternating series), the limit becomes tiny and can reject honest cross-hardware differences that are small relative to the input/error scale. The 102 M4 measurement does not cover that shape. Could we scale each compared quantity by an appropriate scale (or retain a task data scale) and add a near-zero-forecast/high-residual comparison test? I would treat this as a correctness/verification boundary, not just a wording detail.\n\n/cc @rvnminers-A-and-N

This scale also governs MAE/RMSE, step errors and backtest residuals, not just forecast values. For a valid series whose forecast is near zero but whose values/residuals are large (for example, a strongly alternating series), the limit becomes tiny and can reject honest cross-hardware differences that are small relative to the input/error scale. The 102 M4 measurement does not cover that shape. Could we scale each compared quantity by an appropriate scale (or retain a task data scale) and add a near-zero-forecast/high-residual comparison test? I would treat this as a correctness/verification boundary, not just a wording detail.\n\n/cc @rvnminers-A-and-N
Author
Owner

Lior, I'm sorry: I squash-merged at 09:19:24Z. Your inline review landed at 09:19:15Z and your hold request in the same second as the merge, and my last check before merging was a minute or two earlier, so I merged over your hold. Both points are real. I'm fixing them forward now in a follow-up PR:

  1. Advertised capability vs the owner's ceiling. A worker whose --containers-memory-mb is below Chronos-Bolt's 4096 MB will neither offer forecast-chronos:<hash> nor list the Chronos kinds, and it refuses a Chronos task outright. If the container fails for any other reason, the Chronos candidates carry that error and the Go candidates stay usable. Only a cancelled task fails as a whole. There will be tests for a sub-4-GB Confinement.
  2. Tolerance scale. The task output will carry the series' own scale, its largest |value|, computed from the input. Compare checks that scale exactly, so a worker can't widen it. The limit becomes 1e-3 × max(series scale, largest |forecast|), which covers MAE/RMSE, the step errors and the residuals. There will be a near-zero-forecast, large-residual test.

If you'd rather I revert fd0e1a6 first and land it again with these fixes, say so and I'll open the revert PR straight away. I'm holding #668 until this is settled.

Lior, I'm sorry: I squash-merged at 09:19:24Z. Your inline review landed at 09:19:15Z and your hold request in the same second as the merge, and my last check before merging was a minute or two earlier, so I merged over your hold. Both points are real. I'm fixing them forward now in a follow-up PR: 1. **Advertised capability vs the owner's ceiling.** A worker whose `--containers-memory-mb` is below Chronos-Bolt's 4096 MB will neither offer `forecast-chronos:<hash>` nor list the Chronos kinds, and it refuses a Chronos task outright. If the container fails for any other reason, the Chronos candidates carry that error and the Go candidates stay usable. Only a cancelled task fails as a whole. There will be tests for a sub-4-GB `Confinement`. 2. **Tolerance scale.** The task output will carry the series' own scale, its largest |value|, computed from the input. `Compare` checks that scale exactly, so a worker can't widen it. The limit becomes 1e-3 × max(series scale, largest |forecast|), which covers MAE/RMSE, the step errors and the residuals. There will be a near-zero-forecast, large-residual test. If you'd rather I revert `fd0e1a6` first and land it again with these fixes, say so and I'll open the revert PR straight away. I'm holding #668 until this is settled.
@@ -0,0 +310,4 @@
if !f.Containers {
return nil
}
return []string{ChronosLabel}
Author
Owner

Agreed. I'm fixing this forward now; see my comment on the PR. I'll link the follow-up PR here when it's up.

Agreed. I'm fixing this forward now; see my comment on the PR. I'll link the follow-up PR here when it's up.
@@ -0,0 +397,4 @@
for _, v := range slices.Concat(p.Forecast, q.Forecast) {
scale = max(scale, math.Abs(v))
}
limit := chronosTolerance * max(scale, 1e-12)
Author
Owner

Agreed. I'm fixing this forward now; see my comment on the PR. I'll link the follow-up PR here when it's up.

Agreed. I'm fixing this forward now; see my comment on the PR. I'll link the follow-up PR here when it's up.
Owner

No need to revert. Fixing forward is fine.

What landed in fd0e1a6 only goes wrong in two narrow cases: a worker whose owner ceiling is under 4 GB, and the near-zero-forecast tolerance case. Neither one can corrupt a result. Your plan covers both points from the review, including the tests for a sub-4-GB Confinement and for a near-zero forecast with large residuals. Link the follow-up here and we'll review it. #668 can carry on once that's in.

The timing was nobody's fault: the hold arrived in the same second as the merge.

No need to revert. Fixing forward is fine. What landed in `fd0e1a6` only goes wrong in two narrow cases: a worker whose owner ceiling is under 4 GB, and the near-zero-forecast tolerance case. Neither one can corrupt a result. Your plan covers both points from the review, including the tests for a sub-4-GB `Confinement` and for a near-zero forecast with large residuals. Link the follow-up here and we'll review it. #668 can carry on once that's in. The timing was nobody's fault: the hold arrived in the same second as the merge.
@@ -0,0 +310,4 @@
if !f.Containers {
return nil
}
return []string{ChronosLabel}
Author
Owner

The fix is up in #669.

The fix is up in #669.
@@ -0,0 +397,4 @@
for _, v := range slices.Concat(p.Forecast, q.Forecast) {
scale = max(scale, math.Abs(v))
}
limit := chronosTolerance * max(scale, 1e-12)
Author
Owner

The fix is up in #669.

The fix is up in #669.
Author
Owner

Thanks, Lior. The follow-up is #669.

Thanks, Lior. The follow-up is #669.
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Sisyphus/Sisyphus#667