An empty monomial ordering block is dropped inside the engine, so selectInSubring counts blocks wrongly and silently returns the wrong columns
Nobody has claimed this yet.
- Dominant language
- Macaulay2
- Stars
- 435
- Forks
- 297
- Avg merge
- 4d 20h
- Merged PRs (30d)
- 11
Description
This issue was triaged from [`bugs/mike/0-empty-monomial-ordering-blocks`](https://github.com/Macaulay2/M2/blob/388c1ff0ce30d83751dea7bc7eac77fdc1305dd7/bugs/mike/0-empty-monomial-ordering-blocks), one of the 857 files removed from the pre-GitHub `bugs/` tree by [`d2c8d27826`](https://github.com/Macaulay2/M2/commit/d2c8d27826) and catalogued in [#36](https://github.com/Macaulay2/M2/issues/36). **The commentary below was written by Claude (Claude Opus 5, via Claude Code)**, not by @d-torrance, whose account posted it -- please weigh it accordingly.
### The original file, verbatim
```text
The answer should be | x y z |, but the Eliminate 0 block is removed during joining.
i1 : R = QQ[x,y,z, MonomialOrder => {Eliminate 0, Eliminate 1}]
o1 = R
o1 : PolynomialRing
i2 : selectInSubring(1,vars R)
o2 = | y z |
1 2
o2 : Matrix R <--- R
My first attempt to fix it was:
Index: monordering.c
===================================================================
--- monordering.c (revision 14174)
+++ monordering.c (working copy)
@@ -332,7 +332,9 @@
case MO_LAURENT_REVLEX:
case MO_REVLEX:
case MO_WEIGHTS:
- return (p->nvars > 0);
+ // it is a bad idea to elide blocks in the ordering, because that changes the number of them, and
+ // functions such as selectInSubring depend on the numbering:
+ // return (p->nvars > 0);
case MO_POSITION_UP:
case MO_POSITION_DOWN:
return 1;
but that broke many tests.
-- Dan
PS:
Here are some useful tests for after fixing the bug:
-- a fix retains null monomial ordering blocks when joining:
R = QQ[x,y,z, MonomialOrder => {Eliminate 0, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1},{-1},{-1}},{{x, y, z}}) )
R = QQ[x,y,z, MonomialOrder => {Eliminate 1, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1},{-1}},{{y, z}}) )
R = QQ[x,y,z, MonomialOrder => {Eliminate 2, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1}},{{z}}) )
```
### Where it stands today
An empty block in a monomial ordering is dropped inside the engine, so `selectInSubring` counts
blocks differently from the way they were written and silently returns the wrong columns.
```m2
i1 : R = QQ[x,y,z, MonomialOrder => {Eliminate 0, Eliminate 1}]
o1 = R
o1 : PolynomialRing
i2 : selectInSubring(1,vars R)
o2 = | y z |
1 2
o2 : Matrix R <-- R
```
`Eliminate 0` eliminates nothing, so the first block imposes no condition and `selectInSubring(1, ...)`
should return all three variables. Instead it returns the answer belonging to the *second* block —
identical to what a ring with only `{Eliminate 1}` gives.
### Where it goes wrong
The top-level monoid keeps the empty block:
```m2
i1 : R = QQ[x,y,z, MonomialOrder => {Eliminate 0, Eliminate 1}];
i2 : (monoid R).Options.MonomialOrder
o2 = {MonomialSize => 32 }
{Weights => {} }
{Weights => {1} }
{GRevLex => {1, 1, 1}}
{Position => Up }
```
So this is not visible from the monoid. But `selectInSubring` does not consult it —
[`matrix2.m2:700-702`](https://github.com/Macaulay2/M2/blob/development/M2/Macaulay2/m2/matrix2.m2#L700-L702)
hands the question straight to the engine as `rawEliminateVariables(i, m.RawMatrix)`, and the engine
filters blocks through `is_good`, which rejects any block covering no variables:
https://github.com/Macaulay2/M2/blob/development/M2/Macaulay2/e/interface/monomial-ordering.cpp#L66-L90
`is_good` gates both the block count and the block iteration (lines 219 and 230, and again at 658 and
669), so the empty `Weights => {}` block is absent from the engine's numbering and every later block
shifts down by one.
### Tests
These come from the bug file, written by Dan. The first fails today; the second and third pass and are
useful as controls, since they differ only in the first block being non-empty:
```m2
R = QQ[x,y,z, MonomialOrder => {Eliminate 0, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1},{-1},{-1}},{{x, y, z}}) )
R = QQ[x,y,z, MonomialOrder => {Eliminate 1, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1},{-1}},{{y, z}}) )
R = QQ[x,y,z, MonomialOrder => {Eliminate 2, Eliminate 1}]
assert( (selectInSubring(1,vars R)) === map(R^1,R^{{-1}},{{z}}) )
```
### A fix that was already tried
The bug file records an attempt at the obvious change — commenting out the `nvars > 0` test so that
empty blocks survive — with the note that it *"broke many tests"*, and the reasoning:
> it is a bad idea to elide blocks in the ordering, because that changes the number of them, and
> functions such as `selectInSubring` depend on the numbering
So the elision is load-bearing somewhere else, and preserving empty blocks wholesale is not the fix.
Reconciling the two numberings, or rejecting an ordering whose blocks cannot be represented, may be
closer.
Not the same as [#883](https://github.com/Macaulay2/M2/issues/883), which is about `selectInSubring`
being under-documented — the term "block" not being defined — and a proposal to replace it with
something clearer. That issue would not be closed by fixing this one.
Adjacent but distinct from [#4593](https://github.com/Macaulay2/M2/issues/4593), which is also about
how `MonomialOrder` blocks are validated: there an over-long weight vector is rejected outside
`MonomialOrder` and silently accepted inside it. Empty blocks appear in that report only as
scaffolding in the examples, and it concerns neither block numbering nor `selectInSubring`. The two
are worth reading together if someone revisits how blocks are checked and stored.
### Where this came from
Cataloguing the `bugs/` directory removed in d2c8d27826 (#36).
`bugs/mike/0-empty-monomial-ordering-blocks` is Dan's write-up, and it opens with the diagnosis this
issue confirms: *"The answer should be `| x y z |`, but the Eliminate 0 block is removed during
joining."*
`open` · disposition `issue` · source of truth: [`bug-triage/catalog.tsv`](https://github.com/d-torrance/M2/blob/bug-triage/bug-triage/catalog.tsv)
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with matrix2.m2 lines 700-702 and the rawEliminateVariables entry point, then inspect is_good and the block-counting iterations in monomial-ordering.cpp. Run the three listed selectInSubring assertions, including the failing Eliminate 0 case; done means empty blocks no longer shift later block numbering without breaking the control cases and existing tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100