Skip to content

SPARK-1099:Spark's local mode should probably respect spark.cores.max by default - #110

Closed
qqsun8819 wants to merge 3 commits into
apache:masterfrom
qqsun8819:local-cores
Closed

SPARK-1099:Spark's local mode should probably respect spark.cores.max by default#110
qqsun8819 wants to merge 3 commits into
apache:masterfrom
qqsun8819:local-cores

Conversation

@qqsun8819

Copy link
Copy Markdown

This is for JIRA:https://spark-project.atlassian.net/browse/SPARK-1099
And this is what I do in this patch (also commented in the JIRA) @aarondav

This is really a behavioral change, so I do this with great caution, and welcome any review advice:

1 I change the "MASTER=local" pattern of create LocalBackEnd . In the past, we passed 1 core to it . now it use a default cores
The reason here is that when someone use spark-shell to start local mode , Repl will use this "MASTER=local" pattern as default.
So if one also specify cores in the spark-shell command line, it will all go in here. So here pass 1 core is not suitalbe reponding to our change here.
2 In the LocalBackEnd , the "totalCores" variable are fetched following a different rule(in the past it just take in a userd passed cores, like 1 in "MASTER=local" pattern, 2 in "MASTER=local[2]" pattern"
rules:
a The second argument of LocalBackEnd 's constructor indicating cores have a default value which is Int.MaxValue. If user didn't pass it , its first default value is Int.MaxValue
b In getMaxCores, we first compare the former value to Int.MaxValue. if it's not equal, we think that user has passed their desired value, so just use it
c. If b is not satified, we then get cores from spark.cores.max, and we get real logical cores from Runtime. And if cores specified by spark.cores.max is bigger than logical cores, we use logical cores, otherwise we use spark.cores.max
3 In SparkContextSchedulerCreationSuite 's test("local") case, assertion is modified from 1 to logical cores, because "MASTER=local" pattern use default vaules.

qqsun8819 added 2 commits March 9, 2014 14:19
…pecified cores when no cores are passed to it
… construct LocalBackEnd , for use of spark-shell and cores specified in cmd line 2 some test case change from local to local[1]. 3 SparkContextSchedulerCreationSuite test spark.cores.max config in local pattern
@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@ash211

Copy link
Copy Markdown
Contributor

I find that new users often wonder why Spark is only using 1 core, and it's because they expected local to use all their cores rather than defaulting to just one. Changing the default to use all the cores available on the machine when MASTER=local makes sense to me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could simplify the logic by simply putting the default behavior here. Something like

// Use all cores available, up to user-specified limit
val realCores = Runtime.getRuntime.availableProcessors()
val numCores = math.min(realCores, conf.getInt("spark.cores.max", realCores))
val backend = new LocalBackend(scheduler, numCores)

This would allow us to avoid changing much in LocalBackend.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aarondav I'll update the patch accroding to your review

@qqsun8819

Copy link
Copy Markdown
Author

modify patch according to @aarondav 's review

…res and pass it to original LocalBackend constructor
@aarondav

Copy link
Copy Markdown
Contributor

This looks good to me, but I will leave this PR for a little longer in case anyone wants to raise questions about changing the behavior here.

@qqsun8819

Copy link
Copy Markdown
Author

thanks @aarondav It doesn't matter. And welcome any advice for this patch

@aarondav

Copy link
Copy Markdown
Contributor

Oops, I screwed up here and never let Jenkins test. I had to revert it because of a build break. I will reopen it in #182.

@qqsun8819
qqsun8819 deleted the local-cores branch April 30, 2016 03:31
mccheah pushed a commit to mccheah/spark that referenced this pull request Mar 16, 2017
cenyuhai added a commit to cenyuhai/spark that referenced this pull request Oct 8, 2017
[SPARK-21774] 数字类型和字符串比较的时候都统一转成double类型进行比较
现在字符串和数值的比较都是把字符串转成跟数值一样的数据格式之后再去比较 测试case:
`select "1.1" = 1;` `"1.1" = 1`这样的判断,如果是把1.1转成int类型之后就是1了,它就和1相等了... resolveapache#110
See merge request !67
Igosuki pushed a commit to Adikteev/spark that referenced this pull request Jul 31, 2018
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
- Improve the `sed` statement to avoid using specific line number.
- Remove the healthcheck woorkaround since it has been fixed in upstream
- Extend the time to wait LB deployment.
- Do not need collect-k8s-logs role
fishcus pushed a commit to fishcus/spark that referenced this pull request Jun 12, 2020
wangyum added a commit that referenced this pull request Apr 7, 2022
### What changes were proposed in this pull request?
Use sideBySide to format the log plan in `AdaptiveSparkPlanExec`.
Before:
```
12:08:36.876 ERROR org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanExec: Plan changed from SortMergeJoin [key#13], [a#23], Inner
:- Sort [key#13 ASC NULLS FIRST], false, 0
: +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key#13, 5), ENSURE_REQUIREMENTS, [id=#110]
: +- *(1) Filter (isnotnull(value#14) AND (value#14 = 1))
: +- *(1) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false, true) AS value#14]
: +- Scan[obj#12]
+- Sort [a#23 ASC NULLS FIRST], false, 0
+- ShuffleQueryStage 1
+- Exchange hashpartitioning(a#23, 5), ENSURE_REQUIREMENTS, [id=#129]
+- *(2) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).a AS a#23, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).b AS b#24]
+- Scan[obj#22]
to BroadcastHashJoin [key#13], [a#23], Inner, BuildLeft, false
:- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, false] as bigint)),false), [id=#145]
: +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key#13, 5), ENSURE_REQUIREMENTS, [id=#110]
: +- *(1) Filter (isnotnull(value#14) AND (value#14 = 1))
: +- *(1) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false, true) AS value#14]
: +- Scan[obj#12]
+- ShuffleQueryStage 1
+- Exchange hashpartitioning(a#23, 5), ENSURE_REQUIREMENTS, [id=#129]
+- *(2) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).a AS a#23, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).b AS b#24]
+- Scan[obj#22]
```
After:
```
15:57:59.481 ERROR org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanExec: Plan changed:
!SortMergeJoin [key#13], [a#23], Inner BroadcastHashJoin [key#13], [a#23], Inner, BuildLeft, false
!:- Sort [key#13 ASC NULLS FIRST], false, 0 :- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, false] as bigint)),false), [id=#145]
: +- ShuffleQueryStage 0 : +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key#13, 5), ENSURE_REQUIREMENTS, [id=#110] : +- Exchange hashpartitioning(key#13, 5), ENSURE_REQUIREMENTS, [id=#110]
: +- *(1) Filter (isnotnull(value#14) AND (value#14 = 1)) : +- *(1) Filter (isnotnull(value#14) AND (value#14 = 1))
: +- *(1) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false, true) AS value#14] : +- *(1) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).key AS key#13, staticinvoke(class org.apache.spark.unsafe.types.UTF8String, StringType, fromString, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData, true])).value, true, false, true) AS value#14]
: +- Scan[obj#12] : +- Scan[obj#12]
!+- Sort [a#23 ASC NULLS FIRST], false, 0 +- ShuffleQueryStage 1
! +- ShuffleQueryStage 1 +- Exchange hashpartitioning(a#23, 5), ENSURE_REQUIREMENTS, [id=#129]
! +- Exchange hashpartitioning(a#23, 5), ENSURE_REQUIREMENTS, [id=#129] +- *(2) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).a AS a#23, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).b AS b#24]
! +- *(2) SerializeFromObject [knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).a AS a#23, knownnotnull(assertnotnull(input[0, org.apache.spark.sql.test.SQLTestData$TestData2, true])).b AS b#24] +- Scan[obj#22]
! +- Scan[obj#22]
```
### Why are the changes needed?
Enhance readability.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Manual testing.
Closes#36045 from wangyum/SPARK-38772.
Authored-by: Yuming Wang <yumwang@ebay.com>
Signed-off-by: Yuming Wang <yumwang@ebay.com>
cloud-fan pushed a commit that referenced this pull request Apr 20, 2022
### What changes were proposed in this pull request?
This PR adds a new optimizer rule `MergeScalarSubqueries` to merge multiple non-correlated `ScalarSubquery`s to compute multiple scalar values once.
E.g. the following query:
```
SELECT
(SELECT avg(a) FROM t),
(SELECT sum(b) FROM t)
```
is optimized from:
```
== Optimized Logical Plan ==
Project [scalar-subquery#242 [] AS scalarsubquery()#253, scalar-subquery#243 [] AS scalarsubquery()#254L]
: :- Aggregate [avg(a#244) AS avg(a)#247]
: : +- Project [a#244]
: : +- Relation default.t[a#244,b#245] parquet
: +- Aggregate [sum(a#251) AS sum(a)#250L]
: +- Project [a#251]
: +- Relation default.t[a#251,b#252] parquet
+- OneRowRelation
```
to:
```
== Optimized Logical Plan ==
Project [scalar-subquery#242 [].avg(a) AS scalarsubquery()#253, scalar-subquery#243 [].sum(a) AS scalarsubquery()#254L]
: :- Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
: : +- Aggregate [avg(a#244) AS avg(a)#247, sum(a#244) AS sum(a)#250L]
: : +- Project [a#244]
: : +- Relation default.t[a#244,b#245] parquet
: +- Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
: +- Aggregate [avg(a#244) AS avg(a)#247, sum(a#244) AS sum(a)#250L]
: +- Project [a#244]
: +- Relation default.t[a#244,b#245] parquet
+- OneRowRelation
```
and in the physical plan subqueries are reused:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
*(1) Project [Subquery subquery#242, [id=#113].avg(a) AS scalarsubquery()#253, ReusedSubquery Subquery subquery#242, [id=#113].sum(a) AS scalarsubquery()#254L]
: :- Subquery subquery#242, [id=#113]
: : +- AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
*(2) Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
+- *(2) HashAggregate(keys=[], functions=[avg(a#244), sum(a#244)], output=[avg(a)#247, sum(a)#250L])
+- ShuffleQueryStage 0
+- Exchange SinglePartition, ENSURE_REQUIREMENTS, [id=#158]
+- *(1) HashAggregate(keys=[], functions=[partial_avg(a#244), partial_sum(a#244)], output=[sum#262, count#263L, sum#264L])
+- *(1) ColumnarToRow
+- FileScan parquet default.t[a#244] Batched: true, DataFilters: [], Format: Parquet, Location: ..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:int>
+- == Initial Plan ==
Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
+- HashAggregate(keys=[], functions=[avg(a#244), sum(a#244)], output=[avg(a)#247, sum(a)#250L])
+- Exchange SinglePartition, ENSURE_REQUIREMENTS, [id=#110]
+- HashAggregate(keys=[], functions=[partial_avg(a#244), partial_sum(a#244)], output=[sum#262, count#263L, sum#264L])
+- FileScan parquet default.t[a#244] Batched: true, DataFilters: [], Format: Parquet, Location: ..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:int>
: +- ReusedSubquery Subquery subquery#242, [id=#113]
+- *(1) Scan OneRowRelation[]
+- == Initial Plan ==
...
```
Please note that the above simple example could be easily optimized into a common select expression without reuse node, but this PR can handle more complex queries as well.
### Why are the changes needed?
Performance improvement.
```
[info] TPCDS Snappy: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
[info] ------------------------------------------------------------------------------------------------------------------------
[info] q9 - MergeScalarSubqueries off 50798 52521 1423 0.0 Infinity 1.0X
[info] q9 - MergeScalarSubqueries on 19484 19675 226 0.0 Infinity 2.6X
[info] TPCDS Snappy: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
[info] ------------------------------------------------------------------------------------------------------------------------
[info] q9b - MergeScalarSubqueries off 15430 17803 NaN 0.0 Infinity 1.0X
[info] q9b - MergeScalarSubqueries on 3862 4002 196 0.0 Infinity 4.0X
```
Please find `q9b` in the description of SPARK-34079. It is a variant of [q9.sql](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q9.sql) using CTE.
The performance improvement in case of `q9` comes from merging 15 subqueries into 5 and in case of `q9b` it comes from merging 5 subqueries into 1.
### Does this PR introduce _any_ user-facing change?
No. But this optimization can be disabled with `spark.sql.optimizer.excludedRules` config.
### How was this patch tested?
Existing and new UTs.
Closes#32298 from peter-toth/SPARK-34079-multi-column-scalar-subquery.
Lead-authored-by: Peter Toth <peter.toth@gmail.com>
Co-authored-by: attilapiros <piros.attila.zsolt@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
cloud-fan pushed a commit that referenced this pull request Apr 20, 2022
### What changes were proposed in this pull request?
This PR adds a new optimizer rule `MergeScalarSubqueries` to merge multiple non-correlated `ScalarSubquery`s to compute multiple scalar values once.
E.g. the following query:
```
SELECT
(SELECT avg(a) FROM t),
(SELECT sum(b) FROM t)
```
is optimized from:
```
== Optimized Logical Plan ==
Project [scalar-subquery#242 [] AS scalarsubquery()#253, scalar-subquery#243 [] AS scalarsubquery()#254L]
: :- Aggregate [avg(a#244) AS avg(a)#247]
: : +- Project [a#244]
: : +- Relation default.t[a#244,b#245] parquet
: +- Aggregate [sum(a#251) AS sum(a)#250L]
: +- Project [a#251]
: +- Relation default.t[a#251,b#252] parquet
+- OneRowRelation
```
to:
```
== Optimized Logical Plan ==
Project [scalar-subquery#242 [].avg(a) AS scalarsubquery()#253, scalar-subquery#243 [].sum(a) AS scalarsubquery()#254L]
: :- Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
: : +- Aggregate [avg(a#244) AS avg(a)#247, sum(a#244) AS sum(a)#250L]
: : +- Project [a#244]
: : +- Relation default.t[a#244,b#245] parquet
: +- Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
: +- Aggregate [avg(a#244) AS avg(a)#247, sum(a#244) AS sum(a)#250L]
: +- Project [a#244]
: +- Relation default.t[a#244,b#245] parquet
+- OneRowRelation
```
and in the physical plan subqueries are reused:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
*(1) Project [Subquery subquery#242, [id=#113].avg(a) AS scalarsubquery()#253, ReusedSubquery Subquery subquery#242, [id=#113].sum(a) AS scalarsubquery()#254L]
: :- Subquery subquery#242, [id=#113]
: : +- AdaptiveSparkPlan isFinalPlan=true
+- == Final Plan ==
*(2) Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
+- *(2) HashAggregate(keys=[], functions=[avg(a#244), sum(a#244)], output=[avg(a)#247, sum(a)#250L])
+- ShuffleQueryStage 0
+- Exchange SinglePartition, ENSURE_REQUIREMENTS, [id=#158]
+- *(1) HashAggregate(keys=[], functions=[partial_avg(a#244), partial_sum(a#244)], output=[sum#262, count#263L, sum#264L])
+- *(1) ColumnarToRow
+- FileScan parquet default.t[a#244] Batched: true, DataFilters: [], Format: Parquet, Location: ..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:int>
+- == Initial Plan ==
Project [named_struct(avg(a), avg(a)#247, sum(a), sum(a)#250L) AS mergedValue#260]
+- HashAggregate(keys=[], functions=[avg(a#244), sum(a#244)], output=[avg(a)#247, sum(a)#250L])
+- Exchange SinglePartition, ENSURE_REQUIREMENTS, [id=#110]
+- HashAggregate(keys=[], functions=[partial_avg(a#244), partial_sum(a#244)], output=[sum#262, count#263L, sum#264L])
+- FileScan parquet default.t[a#244] Batched: true, DataFilters: [], Format: Parquet, Location: ..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:int>
: +- ReusedSubquery Subquery subquery#242, [id=#113]
+- *(1) Scan OneRowRelation[]
+- == Initial Plan ==
...
```
Please note that the above simple example could be easily optimized into a common select expression without reuse node, but this PR can handle more complex queries as well.
### Why are the changes needed?
Performance improvement.
```
[info] TPCDS Snappy: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
[info] ------------------------------------------------------------------------------------------------------------------------
[info] q9 - MergeScalarSubqueries off 50798 52521 1423 0.0 Infinity 1.0X
[info] q9 - MergeScalarSubqueries on 19484 19675 226 0.0 Infinity 2.6X
[info] TPCDS Snappy: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
[info] ------------------------------------------------------------------------------------------------------------------------
[info] q9b - MergeScalarSubqueries off 15430 17803 NaN 0.0 Infinity 1.0X
[info] q9b - MergeScalarSubqueries on 3862 4002 196 0.0 Infinity 4.0X
```
Please find `q9b` in the description of SPARK-34079. It is a variant of [q9.sql](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q9.sql) using CTE.
The performance improvement in case of `q9` comes from merging 15 subqueries into 5 and in case of `q9b` it comes from merging 5 subqueries into 1.
### Does this PR introduce _any_ user-facing change?
No. But this optimization can be disabled with `spark.sql.optimizer.excludedRules` config.
### How was this patch tested?
Existing and new UTs.
Closes#32298 from peter-toth/SPARK-34079-multi-column-scalar-subquery.
Lead-authored-by: Peter Toth <peter.toth@gmail.com>
Co-authored-by: attilapiros <piros.attila.zsolt@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit e00b81e)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 4, 2026
### What changes were proposed in this pull request?
`VarkaLoopEmitter.Analysis.requireLiteralOffset` is called for `next_day`'s weekday and for `add_months`' month count, and its message was hard-coded to the first: an `AddMonths` over a column month count was rejected as "next_day's weekday must be a literal slot". The check now takes the position it guards, the way `requireValue(node, position)` beside it already does, and each caller names its own: "next_day's weekday" and "add_months' month count". One test in `VarkaLoopEmitterSuite` asserts the `add_months` case is rejected with a message naming `add_months`, so the two cannot drift apart again.
Found by the IR fuzzer's first run (apache#110): its failure quoted the emitter's message, which pointed at a node the shape did not contain.
### Why are the changes needed?
The compiler enforces the rule upstream, so this message only ever reaches a developer driving the emitter directly - which is exactly who cannot afford a message naming the wrong node.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
`catalyst/testOnly *VarkaLoopEmitterSuite *VarkaIrFuzzSuite` where the fuzzer exists, the new test included; `dev/lint-java`, `dev/scalastyle`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 4, 2026
…te, and a nightly
### What changes were proposed in this pull request?
Three more pieces of tooling, stacked on apache#110 (merge apache#108, apache#109 and apache#110 first). Scripts, a template and the two paragraphs that say when to use them.
| piece | what it does |
|---|---|
| `dev/varka_bench_diff.py --requote` | Under each row a regeneration moved, every document line (plans, `SKILLS.md`, docs, README) that quotes the row's old number. A regeneration is done when that list is empty or every remaining line says on purpose that it quotes the number a change moved away from. On task 54's file it finds the two lines in `PLAN_TASK_45.md` that quote `year`'s 2769.1 as that task's own result - correctly historical, and now visible rather than found by grep and memory. |
| `dev/varka_task_new.sh <n> "<title>"` + `sql/varka/plans/TEMPLATE_TASK.md` | Starts a task the way every task here starts: a worktree and branch off master, the plan file from the template, the pre-commit hook installed. The template carries the sections the plans have converged on (compare tasks 53 and 54) with guidance under each to delete as it is filled: where it came from, the admission check done, the design with the switch-and-reference-variant convention, registered op counts on the suite's metric, files, tests and what each is for, the measurement with predictions before the run and the rule that decides the default, risks, sequencing, outcome with predictions scored. It refuses to reuse a branch or overwrite a plan. Self-tested with a throwaway task number and removed. |
| `dev/varka_nightly.sh` | The checks that need volume or an idle machine in one command with a dated log under `target/varka-nightly/`: the canary, the IR fuzzer at ten thousand iterations with the day's seed (`--iterations`, `--seed` to override), the exhaustive calendar sweeps (`--skip-sweep`), optionally the whole gate (`--gate`). The summary names the seed so a failure replays by seed and iteration. Trialled at 1000 iterations with seed 11: canary ok, fuzzer green. |
### Why are the changes needed?
The requote was the last manual step of every regeneration this week; the plan structure was re-derived from earlier plans at the start of every task, and a template is also what lets a cheaper agent produce a plan that reads like the others; and the fuzzer's value scales with iterations nobody wants to wait for interactively.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
`--requote` over master's parity file against task 54's; the scaffold end to end on task 999 in a scratch directory, then removed; the nightly with `--iterations 1000 --seed 11 --skip-sweep`; `dev/varka_precommit.sh --working-tree` clean.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 6, 2026
The review of PR apache#128 found two ways the boundary between what the compiler
assumes and what the emitter guarantees had come apart, and this addresses
both, with the coverage, diagnostics and documentation the same review
found, and a full regeneration of both benchmarks so the committed numbers
describe this code.
The composition hole. `dayRange` had a verdict, `ColumnShifted`, meaning "a
guarded producer is below, so admit", and `admitCalendar` admitted it with
no range test. Sound only while that producer is the calendar node's direct
child - and task 60 added a node that shifts a day by up to
31 * MONTH_ARITH_MAX_MONTHS above one. `year(add_months(date_add(d, off),
m))` fused and answered year 87585 where the truth is -14848: each runtime
guard passed on its own operand and nothing checked the composition. A
guarded producer now contributes the interval its guard establishes,
Bounded(NARROW_MIN_DAYS, NARROW_MAX_DAYS), so every existing rule shifts it
and the result is tested; `ColumnShifted` has no producer left and is gone.
The option the compiler could not see. The count guard sat behind
`guardDayProducers` while `dayRange` bounded a column count unconditionally,
so with the option off the guard vanished and the bound stayed - a wrong
answer, not a reference variant. It moves to `selfGuarding`, whose criterion
it always met: it protects its own magic multiply rather than insuring a
consumer. That makes the option's name accurate again, and makes the
committed guard-on/guard-off A/B degenerate, so the parity pair is now the
column count against a literal count on the same runner, adjacent.
Correct in both directions, and tighter than needed upward: `last_day`,
`next_day`, a positive literal `date_add` and `weekofyear` over a column
offset now decline although they are exact. Pinned by a test that task 69
exists to flip; the downward siblings decline correctly and were wrong
before, on master too.
Also from the review: the operand name restored to `requireOffsetShape`,
whose single message for four operands is what sent the IR fuzzer's first
failure (apache#110) hunting the wrong node, with the assertion that it fires at
all - the test that covered it had been rewritten to `assert(bytes.nonEmpty)`,
which passes on any emit; a live violation driven through the masked
epilogue, the body this task's VerifyError surfaced in and the one no test
reached; the `ownWord == false` branch covered; the differential's headline
test split so the kernel actually runs, since its eight rows are one batch
and two tripped the guard, making it compare the row engine with itself; the
fuzzer given a small-magnitude column so a column month count is reachable;
`canonical()`'s missing `truncDate`, which collided two variants on one
shape hash; an `add_months(d, smallint)` reason that named the wrong thing;
and a `CASE`/`IF` fusion cliff registered rather than fixed, since both fixes
move bytes.
`dev/varka_quote_check.py` gains `--full-history`. Resolving a results file
with `git checkout --ours` leaves the merge TREESAME to one parent, so git
prunes the other's whole line for that path and every version it wrote
becomes invisible: 16 numbers quoted in PLAN_TASK_37/42/57/58/59 read as
orphans on this branch against 0 on master, for numbers that are committed
and correct. The flag keeps the promise the tool's docstring makes, costs
nothing measurable, and still catches a fabricated number.
Benchmarks regenerated at both widths on the idle machine under the
performance governor - parity at load 0.61, throughput at 0.59, canaries
within 2.1%. The column form costs -4.7% null-free and -7.9% mixed at 256
bits, -5.6% and -13.4% at 128, against the literal control; that delta is
the guard plus one column load in place of a broadcast and cannot be split
further without reintroducing the coupling this review removed. The second
stream is free: 727.3 against the one-stream 727.5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3VxmqDKhWHkvQ4Jrp6coG
Sign up for freeto 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.

4 participants

@qqsun8819@AmplabJenkins@ash211@aarondav