Discussion:
[Maxima-commits] [git] Maxima CAS branch master updated. branch-5_40-base-56-g4401f6d
(too old to reply)
Kris Katterjohn
2017-06-22 22:53:21 UTC
Permalink
Raw Message
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Maxima CAS".
<snip>
- Log -----------------------------------------------------------------
commit 4401f6d77fae47e1e5b942c86fcc5f3458519b5a
Date: Thu Jun 15 11:57:26 2017 +0200
replaced ml-type with typeof test or arrayp respectively
<snip>
diff --git a/share/affine/polya.lisp b/share/affine/polya.lisp
index 61b0dcb..5a50f3b 100644
--- a/share/affine/polya.lisp
+++ b/share/affine/polya.lisp
<snip>
@@ -2447,7 +2447,7 @@ dot_products, much the same as can be obtained by doing $dotsimp")
(defvar *show-entry-type* t)
(defun $determinant_of_equations (eqn &optional variables &aux answer )
- (cond ((ml-typep $poly_vector 'polynomial-vectors) nil)
+ (cond ((eq (typeof $poly_vector) 'polynomial-vectors) nil)
(t (setq $poly_vector (make-polynomial-vectors))))
(setf (pv-type-of-entries $poly_vector) $type_of_entries_for_poly_vector)
(cond (variables ( pv-get-rows-from-macsyma-equations-and-variables $poly_vector
diff --git a/share/affine/sparsemat.lisp b/share/affine/sparsemat.lisp
index 9726ecd..990d99f 100644
--- a/share/affine/sparsemat.lisp
+++ b/share/affine/sparsemat.lisp
@@ -1133,8 +1133,7 @@ something is wrong" (length (sp-list-of-all-columns-occurring sp-mat)) number-of
(vector-push
(aref (sp-column-used-in-row sp-mat) nn) a-special-solution)
(vector-push a-new-entry a-special-solution)))))))
- (cond ((and (sp-solutions sp-mat)(ml-typep
- (sp-solutions sp-mat) 'sparse-matrix)))
+ (cond ((and (sp-solutions sp-mat) (eq (typeof (sp-solutions sp-mat)) 'sparse-matrix))))
(t (setf (sp-solutions sp-mat) (make-sparse-matrix ))))
(sp-set-rows (sp-solutions sp-mat) solution-rows)
(sp-set-type-of-entries (sp-solutions sp-mat)
You added an extra closing parenthesis and you used typeof instead of
type-of. I went ahead and removed the extra parenthesis and changed
typeof to type-of since I happened to stumble across this (although
perhaps the struct type predicates are more appropriate here).

Now the affine package can at least be loaded without causing an error
(from the extra parenthesis). I did some basic tests with fast_linsolve
and determinant_of_equations and it seems to work as expected now
without causing lisp errors (from the nonexistent typeof). (I compared
the results to the results from Maxima 5.38.)

You've been making a bunch of changes to src and share, so I'm wondering
what sorts of testing you've done regarding these changes and if you
plan to do more testing.

Cheers,
Kris Katterjohn
Andreas Eder
2017-06-23 06:40:36 UTC
Permalink
Raw Message
Post by Kris Katterjohn
You've been making a bunch of changes to src and share, so I'm
wondering
what sorts of testing you've done regarding these changes and if you
plan to do more testing.
I have done quite a lot of testing besides running the testsuite,
but
this one seems to have slipped through. I'm glad you have found
the
typo.
Thank you very much.

Andreas
--
ceterum censeo redmondinem esse delendam
Raymond Toy
2017-06-23 20:25:50 UTC
Permalink
Raw Message
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Maxima CAS".
Kris> <snip>
- Log -----------------------------------------------------------------
commit 4401f6d77fae47e1e5b942c86fcc5f3458519b5a
Date: Thu Jun 15 11:57:26 2017 +0200
replaced ml-type with typeof test or arrayp respectively
Kris> <snip>
diff --git a/share/affine/polya.lisp b/share/affine/polya.lisp
index 61b0dcb..5a50f3b 100644
--- a/share/affine/polya.lisp
+++ b/share/affine/polya.lisp
Kris> <snip>
@@ -2447,7 +2447,7 @@ dot_products, much the same as can be obtained by doing $dotsimp")
(defvar *show-entry-type* t)
(defun $determinant_of_equations (eqn &optional variables &aux answer )
- (cond ((ml-typep $poly_vector 'polynomial-vectors) nil)
+ (cond ((eq (typeof $poly_vector) 'polynomial-vectors) nil)
(t (setq $poly_vector (make-polynomial-vectors))))
(setf (pv-type-of-entries $poly_vector) $type_of_entries_for_poly_vector)
(cond (variables ( pv-get-rows-from-macsyma-equations-and-variables $poly_vector
diff --git a/share/affine/sparsemat.lisp b/share/affine/sparsemat.lisp
index 9726ecd..990d99f 100644
--- a/share/affine/sparsemat.lisp
+++ b/share/affine/sparsemat.lisp
@@ -1133,8 +1133,7 @@ something is wrong" (length (sp-list-of-all-columns-occurring sp-mat)) number-of
(vector-push
(aref (sp-column-used-in-row sp-mat) nn) a-special-solution)
(vector-push a-new-entry a-special-solution)))))))
- (cond ((and (sp-solutions sp-mat)(ml-typep
- (sp-solutions sp-mat) 'sparse-matrix)))
+ (cond ((and (sp-solutions sp-mat) (eq (typeof (sp-solutions sp-mat)) 'sparse-matrix))))
(t (setf (sp-solutions sp-mat) (make-sparse-matrix ))))
(sp-set-rows (sp-solutions sp-mat) solution-rows)
(sp-set-type-of-entries (sp-solutions sp-mat)
Kris> You added an extra closing parenthesis and you used typeof instead of
Kris> type-of. I went ahead and removed the extra parenthesis and changed
Kris> typeof to type-of since I happened to stumble across this (although
Kris> perhaps the struct type predicates are more appropriate here).

On the other hand, is there something wrong with ml-typep? The bad
thing about it is that it's not obvious what it does. The best thing
about it is that you only need to understand it once to know what it's
doing wherever it's being used.

And the fact that it didn't show up in our test suite says we need a
test that actually tests for these changes.

That, more than anything is, is the most important part. We are
making changes that seem harmless but possibly aren't because there
are no tests that actually test it.

That is really bad if we're churning the code like this.

--
Ray
Robert Dodier
2017-06-24 06:14:50 UTC
Permalink
Raw Message
Post by Raymond Toy
And the fact that it didn't show up in our test suite says we need a
test that actually tests for these changes.
That, more than anything is, is the most important part. We are
making changes that seem harmless but possibly aren't because there
are no tests that actually test it.
That is really bad if we're churning the code like this.
Agreed -- changing stuff for which there is no test is dangerous.

best

Robert Dodier

Loading...