John Wiegley <[email protected]> writes:
>>>>>> Ihor Radchenko <[email protected]> writes:
>
>> Not yet converged :)
>> Let this human pass the next input from the mighty LLM.
>> (Jokes aside, iterative convergence of the cells is what I use by
>> myself, and I'd prefer to keep that feature)
>
> Let’s keep the fun going. Many fixes posted to
>
> https://github.com/jwiegley/org-table-loeb
I finally got to this.
Now, testing with my latest version of the custom harness :)
The generated report is below:
Summary: six behavioral divergences, three of which are critical. They
cluster into a small number of root causes in the substitution and
iteration layers.
---
1. Iteration freezes self-referential cells instead of iterating
=org-table-loeb--make-cell-closure= returns the initial value for any cell
inside an SCC:
#+begin_src elisp
((and cycle-cells (gethash (cons row col) cycle-cells) formula)
(lambda (_matrix) value))
#+end_src
So between passes of =org-table-loeb--iterate= the cell value never changes.
Classic recomputes from the previous pass.
Example (converges to a wrong value):
#+begin_example
| 100 | |
#+TBLFM: @1$2=floor(($1+@1$2)/2)
#+end_example
- classic (C-u C-u C-c /): =("100" "99")=
- loeb: =("100" "")=, reports "Convergence after 2 iterations"
Example (divergent):
#+begin_example
| 0 | |
#+TBLFM: @1$1=1+@1$1
#+end_example
- classic: =("10" "")=, then "No convergence after 10 iterations"
- loeb: =("0" "")=, "Table was already stable"
Fix: implement real fixed-point iteration — each pass recomputes every SCC cell
from the previous pass's values. This means SCC cells must not go through the
Löb thunk layer at all; freeze-at-initial-value is not iteration.
---
2. Negative precision flags are ignored
Classic parses =f=, =n=, =s=, =e=, =p= with an optional sign:
=\([pnfse]\)\(-?[0-9]+\)=. Loeb's regex in =org-table-loeb--parse-format-flags=
is =\([pnfse]\)\([0-9]+\)= — no =-?=.
- =@1$2=$1/4;f-1=: classic ="1.3"=, loeb ="1.25"=
- =@1$2=$1/2;f-2=: classic ="2.50"=, loeb ="2.5"=
Also, the =f-1= branch in =org-table-loeb--format-result= ("preserve empty
results") is dead code and encodes a wrong reading: =f-1= means one decimal
place, not "preserve empty". Either fix the parse (add =-?=) and remove the
misleading branch, or drop it.
---
3. Range endpoint with omitted column (breaks the most common sum idiom)
Org's rule: "if you omit the column, the current column is implied."
=org-table-loeb--substitute-ranges= does two things wrong:
- For =@R1$C1..@R2= (end column omitted), it defaults the end column to the
/start/ column: =(c2 (or (match-string 5 result) c1))=.
- It has no case for a bare row range with no columns at all, =@2..@-1=.
Consequences:
#+begin_example
| 10 |
| 20 |
| 30 |
| |
#+TBLFM: @>$1=vsum(@2..@-1)
#+end_example
- classic: =... "50"=
- loeb: =... "#ERROR: Expected a number"=
=vsum(@2..@-1)= is the canonical "sum this column" idiom (top SO answer for
summing a column). This is a hard regression.
Same root cause, with a column on the start only:
#+begin_example
| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 0 | 0 | |
#+TBLFM: @3$3=vsum(@1$1..@3)
#+end_example
- classic: =... "21"= (current column 3 implied)
- loeb: =... "5"=
And with a relative start:
#+begin_example
#+TBLFM: @3$3=vsum(@-1$-2..@-1)
#+end_example
- classic: =... "15"=
- loeb: =... "4"=
Fix: when either endpoint omits its column, substitute the /current/ column
(the column of the target field), not the other endpoint's column; and add a
case for bare row ranges =@R1..@R2=.
Note: your existing =test-org-table-loeb/range-implicit-column= currently
encodes the wrong semantics (it expects the omitted column to mean "same as the
other endpoint", result 60). It blesses the bug and will need to change once
this is fixed.
---
4. Self-reference through the =$>= alias is dropped, not computed
#+begin_example
| 1 | 2 | 3 | 4 | 5 | |
#+TBLFM: $6=$>+$>>
#+end_example
- classic: =... "5"= (=$>= expands to =$6=, reads the pre-existing empty cell
as 0, adds =$>>==5)
- loeb: =... ""= (silently leaves the cell empty)
Same class as #1: =$>= → =$6= makes this a self-cycle, and loeb freezes it. A
single-pass =C-c /= should compute from the current value, not drop the result.
---
5. =L= flag does not interpolate unquoted numeric references
='(substring "$1" $2 $3);L= with =("hello world" "0" "5")=:
- classic: ="hello"=
- loeb: =#ERROR: Wrong type argument: integerp, "0"=
Root cause: =org-table-loeb--substitute-refs= only branches on
=:force-numbers=, never =:literal=. Under =L=, =$2= and =$3= should be
interpolated bare (=0=, =5=), not wrapped in =org-table-loeb--get= (which
returns ="0"=, ="5"=).
---
6. =E= flag does not make quoted ="nan"= comparisons work
=if("$1" == "nan" || "$2" == "nan", string(""), $1 + $2); E= with =("1" "")=:
- classic: =""= (the =if= branch fires)
- loeb: ="nan"= (falls through to =$1+$2=)
Root cause: the empty→ ="nan"= substitution under =E= is not applied inside the
quoted string =""$2""=, so ="$2" == "nan"= is false and Calc computes =1 + nan=.
---
How I reproduced it
Self-contained harness (run against an Org checkout with =org-test= and
=org-table-loeb= on the load path):
#+begin_src elisp
;;; compare-loeb.el --- classic vs loeb differential runner -*-
lexical-binding: t; -*-
(require 'org)
(require 'org-table)
(require 'org-table-loeb)
(org-table-loeb-install)
(defun cmp--result (text engine all)
"Run TEXT through ENGINE with ALL recalc arg; return stripped table as list."
(let ((buf (generate-new-buffer " *cmp*")))
(unwind-protect
(with-current-buffer buf
(org-mode)
(insert text)
(goto-char (point-min))
(let ((org-table-formula-engine engine))
(condition-case err
(org-table-recalculate all)
(error (format "ERROR<%s>" (error-message-string err)))))
(mapcar (lambda (row)
(if (eq row 'hline) 'hline
(mapcar (lambda (c) (if (stringp c)
(substring-no-properties c) c))
row)))
(org-table-to-lisp)))
(kill-buffer buf))))
(defun cmp--case (label text &optional all)
(let* ((all (or all 'all))
(c (cmp--result text 'classic all))
(l (cmp--result text 'loeb all)))
(princ (format "%-28s %s\n" label (if (equal c l) "MATCH" "DIVERGE")))
(unless (equal c l)
(princ (format " classic=%S\n loeb =%S\n" c l)))))
(cmp--case "iter-convergent" "| 100 | |
#+TBLFM: @1$2=floor(($1+@1$2)/2)" 'iterate)
(cmp--case "iter-divergent" "| 0 | |
#+TBLFM: @1$1=1+@1$1" 'iterate)
(cmp--case "f-neg-1" "| 5 | |
#+TBLFM: @1$2=@1$1/4;f-1")
(cmp--case "f-neg-2" "| 5 | |
#+TBLFM: @1$2=@1$1/2;f-2")
(cmp--case "bare-row-range" "| 10 |
| 20 |
| 30 |
| |
#+TBLFM: @>$1=vsum(@2..@-1)")
(cmp--case "end-col-omitted" "| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 0 | 0 | |
#+TBLFM: @3$3=vsum(@1$1..@3)")
(cmp--case "rel-range-end-col" "| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 0 | 0 | |
#+TBLFM: @3$3=vsum(@-1$-2..@-1)")
(cmp--case "self-ref-dollar-gt" "| 1 | 2 | 3 | 4 | 5 | |
#+TBLFM: $6=$>+$>>")
(cmp--case "L-flag-substring" "| hello world | 0 | 5 | |
#+TBLFM: @1$4='(substring \"$1\" $2 $3);L")
(cmp--case "E-nan-if" "| 1 | | |
#+TBLFM: @1$3=if(\"$1\" == \"nan\" || \"$2\" == \"nan\", string(\"\"), $1 +
$2); E f-1")
#+end_src
Run:
#+begin_src bash
emacs -Q --batch \
-L /path/to/org-mode/lisp -L /path/to/org-mode/testing \
-L /path/to/org-table-loeb \
-l compare-loeb.el
#+end_src
---
ERT tests to add to =test-org-table-loeb.el=
Note: your =test-org-table-loeb--compare-engines= macro hardcodes ='all= as the
recalc argument, so it cannot be used for the iteration cases. Either extend it
with an optional =ALL= argument, or add a second macro. Below I assume the
extended form =(... (text) &optional all)=.
#+begin_src elisp
;; A. iteration
(ert-deftest test-org-table-loeb/parity-iterate-convergent ()
(test-org-table-loeb--compare-engines
"| 100 | |
#+TBLFM: @1$2=floor(($1+@1$2)/2)" 'iterate))
(ert-deftest test-org-table-loeb/parity-iterate-divergent ()
(test-org-table-loeb--compare-engines
"| 0 | |
#+TBLFM: @1$1=1+@1$1" 'iterate))
;; B. negative precision
(ert-deftest test-org-table-loeb/parity-f-neg-1 ()
(test-org-table-loeb--compare-engines
"| 5 | |
#+TBLFM: @1$2=@1$1/4;f-1"))
(ert-deftest test-org-table-loeb/parity-f-neg-2 ()
(test-org-table-loeb--compare-engines
"| 5 | |
#+TBLFM: @1$2=@1$1/2;f-2"))
;; C. omitted column in ranges
(ert-deftest test-org-table-loeb/parity-bare-row-range ()
(test-org-table-loeb--compare-engines
"| 10 |
| 20 |
| 30 |
| |
#+TBLFM: @>$1=vsum(@2..@-1)"))
(ert-deftest test-org-table-loeb/parity-end-col-omitted ()
(test-org-table-loeb--compare-engines
"| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 0 | 0 | |
#+TBLFM: @3$3=vsum(@1$1..@3)"))
;; E. L flag literal numbers
(ert-deftest test-org-table-loeb/parity-L-flag-substring ()
(test-org-table-loeb--compare-engines
"| hello world | 0 | 5 | |
#+TBLFM: @1$4='(substring \"$1\" $2 $3);L"))
;; F. E flag + quoted nan comparison
(ert-deftest test-org-table-loeb/parity-E-nan-if ()
(test-org-table-loeb--compare-engines
"| 1 | | |
#+TBLFM: @1$3=if(\"$1\" == \"nan\" || \"$2\" == \"nan\", string(\"\"), $1 +
$2); E"))
#+end_src
---
Additional:
1. The =defcustom= has no =:package-version= and no =:safe= — both required to
be explicit by Org convention.
2. The file Commentary claims "topological sort" and "fall back to iteration
for SCCs"; neither exists (order is thunk-forcing; SCCs either error or
freeze). CLAUDE.md marks #5 "(Reserved)", but the source Commentary does not.
What I did not cover: cross-file and =org-id= remote references, and
=constants.el= unit references (=$c/$1/$cm=), since they need an external
file/package. Worth a follow-up pass.
[Disclaimer: all values are from the sandbox copies at /tmp/scratch/org-mode
(your checkout) and /tmp/scratch/org-table-loeb, compared via
org-table-to-lisp. I did not modify the real tree.]
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
