Bug #834
Fix test failures (after revising MatrixForOrdering)
Description
The current CVS produces 3 failed CoCoALib tests:test-MatrixForOrdering1, test-RingWeyl1, test-SparsePolyRing1
and 2 failed examples:ex-RingWeyl2, ex-RingWeyl3
The following CoCoA-5 tests failed too:demo-Osaka2015.cocoa5 primary.cocoa5 lecture-HF1.cocoa5 lecture-HF4.cocoa5 radical.cocoa5 SourceAnna.cocoa5
Compilation with debugging activated produced 7 failed tests:test-GOperations1, test-GOperations2, test-MatrixForOrdering1, test-QuotientRing1, test-RingWeyl1, test-SparsePolyRing1, test-TmpMorseGraph
and 4 failed examples:ex-module2 ex-PPMonoidElem2 ex-RingWeyl2 ex-RingWeyl3
Related issues
History
#1
Updated by John Abbott over 8 years ago
Updated by JAA believes the root cause is the check that a grading is strictly positive when weakly positive is sufficient.
#2
Updated by Anna Maria Bigatti over 8 years ago
Updated by - Status changed from New to In Progress
- Assignee set to Anna Maria Bigatti
- % Done changed from 0 to 10
First bug trapped:
IsPositiveOrd wants rk(M) == NumRows(M).
This function is called when making a term ordering out of a possibly redundant matrix. Obviously incompatible
(Failure in test-SparsePolyRing1)
I think the check on the columns should be an independent function called by IsPositiveOrd which will also do the extra check on the rank.
.. Need to find a good name for it...
#3
Updated by John Abbott over 8 years ago
I have just checked in a modified version of MatrixForOrdering.C following Anna's suggestion in comment 2.
Now "only" 5 tests fail (with debugging on).
I have added a new fn called ColCheck; I think its interface is reasonable, but I am not happy with its name.
I note that MatrixForOrdering.C still contains a lot of cruft :-/
#4
Updated by Anna Maria Bigatti over 8 years ago
lines 304 and following (#ifdef) in PPOrdering.C should be deleted:
we do non require to have a PositiveGrading for making an ordering. It is only necessary if we want to compute a Hilbert Series. (the check should be there... maybe it isn't... yet)
#5
Updated by Anna Maria Bigatti over 8 years ago
- % Done changed from 10 to 20
I'm writing another function HasNegEntry which checks whether there is a negative entry.
(then we can think of a better name ;-)
#6
Updated by Anna Maria Bigatti over 8 years ago
I'm also writing IsUpperTriangular.
Addendum (JAA) also IsLowerTriangular? Will these go into a "matrix" source file?
#7
Updated by Anna Maria Bigatti over 8 years ago
- Subject changed from Fix test failures (after revising MatrixFroOrdering) to Fix test failures (after revising MatrixForOrdering)
#8
Updated by John Abbott over 8 years ago
- % Done changed from 20 to 10
Ignoring the question of the name for ColCheck, is it better to have a single fn with a flag to say whether or not to allow zero cols, or is it better to have 2 fns (with similar names) one allowing zero cols and the other not?
The current situation is a single fn with a "flag", so a call looks like this:
matrix M; ... if (ColCheck(M, WithoutZeroCol)) ... ... if (ColCheck(M, PossiblyWithZeroCol)) ...
The alternative would look something like this:
matrix M; ... if (ColCheckWithoutZeroCol(M)) ... ... if (ColCheckWithZeroCol(M)) ...
Not sure which I prefer. It would be nice to have a general guideline, but that may be difficult to achieve universally.
Comments? Ideas? Criticisms?
#9
Updated by Anna Maria Bigatti over 8 years ago
I think there are no more failures now. Close this issue?
#10
Updated by Anna Maria Bigatti over 8 years ago
- Status changed from In Progress to Closed
- % Done changed from 10 to 100
All tests pass (no idea how long we have spent on this). Closing.
John Abbott wrote:
Ignoring the question of the name for
ColCheck, is it better to have a single fn with a flag to say whether or not to allow zero cols, or is it better to have 2 fns (with similar names) one allowing zero cols and the other not?
OK with the flag.