bug#25652: 26.0.50; calc says that cos(1 degree) is 0.54

[Tino Calancha <tino.calancha@gmail.com>]

[Forwading to the bug thread: Sorry if you receive this message twice]

npostavs@users.sourceforge.net writes:

> tags 25652 confirmed
> retitle 25652 26.0.50; [calc] In Units Simplication + Degree mode cos(1 deg) != cos(1)
> quit
>
> Torsten Bronger <bronger@physik.rwth-aachen.de> writes:
>>> You can toggle this in calc mode with "m U".  So "m d m U 1 C m U
>>> 1 C" should exhibit the problem.  Note that you have "Deg" active
>>> the whole time.  [...]
>>
>> After havin updated to current git master, I can now reproduce this
>> on both of my machine.  Can you confirm?
>
> Yes, this happens in units simplication mode.  Note that 'cos(1 deg) RET
> does give 0.999.  It seems to be because of the fix for #23889 [1:
> 713e922243].  Tino, any ideas?
>
> 1: 2016-07-12 00:38:14 +0900 713e922243fb60d850f7b0ff83f3e2a3682f1832
>   Ignore angle mode while simplifying units
Yes, my commit produced this bug.  We must revert it.  Sorry for that.
That commit was to fix Bug#23889.  It would be nice if we can fix Bug#23889 in
a new way.
In that bug the problem was that, if the user inputs a symbolic expression containning
an angle unit, then this unit is overriden by calc-angle-mode while simplifying such
expression.
That should not happen as mentioned in the manual:
(info "(calc) Trigonometric and Hyperbolic Functions")

That is,
M-x calc RET
m d ; Set calc-angle-mode to degrees.
' sin (45 deg) RET
u s ; Must insert the value of:  (sin (* 45 pi (/ 180.0)))
;; But instead, inserts the value of: (sin (* 45 (expt (/ pi 180.0) 2)))
;; i.e., it's applying (/ pi 180) twice.

I have updated the patch:
1) Revert 713e922243

2) Bind calc-angle-mode to 'rad while simplifying an expression whenever such
expression contains an unit angle.  This way, hopefully, the second factor (/ pi 180)
won't be applied.

Opinions, comments...
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 1a2592409c0fcfca7826b71248d3d3d234355c13 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 12 Feb 2017 21:14:34 +0900
Subject: [PATCH 1/2] Revert 'Ignore angle mode while simplifying units'

This commit (713e922243) cause regressions (Bug#25652).
---
  lisp/calc/calc-forms.el | 12 +++---------
  lisp/calc/calc-math.el  | 12 ++++--------
  2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/lisp/calc/calc-forms.el b/lisp/calc/calc-forms.el
index 6aa421ec20..abf76cf07e 100644
--- a/lisp/calc/calc-forms.el
+++ b/lisp/calc/calc-forms.el
@@ -317,9 +317,7 @@ math-to-hms
  	 (list 'calcFunc-hms a))
  	((math-negp a)
  	 (math-neg (math-to-hms (math-neg a) ang)))
-	((eq (or ang
-                 (and (not math-simplifying-units) calc-angle-mode))
-                 'rad)
+	((eq (or ang calc-angle-mode) 'rad)
  	 (math-to-hms (math-div a (math-pi-over-180)) 'deg))
  	((memq (car-safe a) '(cplx polar)) a)
  	(t
@@ -356,16 +354,12 @@ math-from-hms
  	   (if (eq (car-safe a) 'sdev)
  	       (math-make-sdev (math-from-hms (nth 1 a) ang)
  			       (math-from-hms (nth 2 a) ang))
-	     (if (eq (or ang
-                         (and (not math-simplifying-units) calc-angle-mode))
-                     'rad)
+	     (if (eq (or ang calc-angle-mode) 'rad)
  		 (list 'calcFunc-rad a)
  	       (list 'calcFunc-deg a)))))
  	((math-negp a)
  	 (math-neg (math-from-hms (math-neg a) ang)))
-	((eq (or ang
-                 (and (not math-simplifying-units) calc-angle-mode))
-             'rad)
+	((eq (or ang calc-angle-mode) 'rad)
  	 (math-mul (math-from-hms a 'deg) (math-pi-over-180)))
  	(t
  	 (math-add (math-div (math-add (math-div (nth 3 a)
diff --git a/lisp/calc/calc-math.el b/lisp/calc/calc-math.el
index 2590761d53..faa318d45d 100644
--- a/lisp/calc/calc-math.el
+++ b/lisp/calc/calc-math.el
@@ -763,14 +763,12 @@ calcFunc-nroot
  (defun math-to-radians (a)   ; [N N]
    (cond ((eq (car-safe a) 'hms)
  	 (math-from-hms a 'rad))
-	((and (not math-simplifying-units)
-              (memq calc-angle-mode '(deg hms)))
+	((memq calc-angle-mode '(deg hms))
  	 (math-mul a (math-pi-over-180)))
  	(t a)))

  (defun math-from-radians (a)   ; [N N]
-  (cond ((and (not math-simplifying-units)
-              (eq calc-angle-mode 'deg))
+  (cond ((eq calc-angle-mode 'deg)
  	 (if (math-constp a)
  	     (math-div a (math-pi-over-180))
  	   (list 'calcFunc-deg a)))
@@ -781,16 +779,14 @@ math-from-radians
  (defun math-to-radians-2 (a &optional force-symbolic)   ; [N N]
    (cond ((eq (car-safe a) 'hms)
  	 (math-from-hms a 'rad))
-	((and (not math-simplifying-units)
-              (memq calc-angle-mode '(deg hms)))
+	((memq calc-angle-mode '(deg hms))
  	 (if (or calc-symbolic-mode force-symbolic)
  	     (math-div (math-mul a '(var pi var-pi)) 180)
  	   (math-mul a (math-pi-over-180))))
  	(t a)))

  (defun math-from-radians-2 (a &optional force-symbolic)   ; [N N]
-  (cond ((and (not math-simplifying-units)
-              (memq calc-angle-mode '(deg hms)))
+  (cond ((memq calc-angle-mode '(deg hms))
  	 (if (or calc-symbolic-mode force-symbolic)
  	     (math-div (math-mul 180 a) '(var pi var-pi))
  	   (math-div a (math-pi-over-180))))
-- 
2.11.0

From b1b6f62f5baa8023a7b5d45b1d30399c8e8f82a2 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 12 Feb 2017 21:45:47 +0900
Subject: [PATCH 2/2] Prevent from override input angle unit while simplifying

Ignore calc-angle-mode while simplifying if the expression
contains angle units (Bug#23899).
* lisp/calc/calc-alg.el (calc-input-angle-units): New defun.
(math-simplify): If TOP-EXPR contains angle units, then bind
calc-angle-mode to 'rad.
* test/lisp/calc/calc-tests.el (test-calc-23889): New test.
---
  lisp/calc/calc-alg.el        |  9 +++++++++
  test/lisp/calc/calc-tests.el | 46 ++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 55 insertions(+)

diff --git a/lisp/calc/calc-alg.el b/lisp/calc/calc-alg.el
index 4e63d238c7..9db901a975 100644
--- a/lisp/calc/calc-alg.el
+++ b/lisp/calc/calc-alg.el
@@ -355,10 +355,19 @@ math-hyperbolic-trig-rewrite
  ;; math-simplify-step, which is called by math-simplify.
  (defvar math-top-only)

+(defun calc-input-angle-units (input)
+  (cond ((math-expr-contains input '(var deg var-deg)) 'deg)
+        ((math-expr-contains input '(var rad var-rad)) 'rad)
+        ((math-expr-contains input '(var hms var-hms)) 'hms)
+        (t nil)))
+
  ;; math-normalize-error is declared in calc.el.
  (defvar math-normalize-error)
  (defun math-simplify (top-expr)
    (let ((math-simplifying t)
+        (calc-angle-mode (if (calc-input-angle-units top-expr)
+                             'rad
+                           calc-angle-mode))
  	(math-top-only (consp calc-simplify-mode))
  	(simp-rules (append (and (calc-has-rules 'var-AlgSimpRules)
  				 '((var AlgSimpRules var-AlgSimpRules)))
diff --git a/test/lisp/calc/calc-tests.el b/test/lisp/calc/calc-tests.el
index 8f56d48d01..45b735c3c6 100644
--- a/test/lisp/calc/calc-tests.el
+++ b/test/lisp/calc/calc-tests.el
@@ -86,6 +86,52 @@ calc-tests-simple
  					       (math-read-expr "1m") "cm")
  			    '(* -100 (var cm var-cm)))))

+(ert-deftest test-calc-23889 ()
+  "Test for http://debbugs.gnu.org/23889 and 25652."
+  (dolist (mode '(deg rad))
+    (let ((calc-angle-mode mode))
+      ;; If user inputs angle units, then should ignore `calc-angle-mode'.
+      (should (string= "5253"
+                       (substring
+                        (number-to-string
+                         (nth 1
+                              (math-simplify-units
+                               '(calcFunc-cos (* 45 (var rad var-rad))))))
+                        0 4)))
+      (should (string= "7071"
+                       (substring
+                        (number-to-string
+                         (nth 1
+                              (math-simplify-units
+                               '(calcFunc-cos (* 45 (var deg var-deg))))))
+                        0 4)))
+      (should (string= "8939"
+                       (substring
+                        (number-to-string
+                         (nth 1
+                              (math-simplify-units
+                               '(+ (calcFunc-sin (* 90 (var rad var-rad)))
+                                   (calcFunc-cos (* 90 (var deg var-deg)))))))
+                        0 4)))
+      (should (string= "5519"
+                       (substring
+                        (number-to-string
+                         (nth 1
+                              (math-simplify-units
+                               '(+ (calcFunc-sin (* 90 (var deg var-deg)))
+                                   (calcFunc-cos (* 90 (var rad var-rad)))))))
+                        0 4)))
+      ;; If user doesn't input units, then must use `calc-angle-mode'.
+      (should (string= (if (eq calc-angle-mode 'deg)
+                           "9998"
+                         "5403")
+                       (substring
+                        (number-to-string
+                         (nth 1
+                              (math-simplify-units
+                               '(calcFunc-cos 1))))
+                        0 4))))))
+
  (provide 'calc-tests)
  ;;; calc-tests.el ends here

-- 
2.11.0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.7)
  of 2017-02-12
Repository revision: 862d6438cfa6c6c035033697751f3d002357b024