Bug #3137

complex.rb changes exceptions of Math

Added by Yusuke Endoh almost 5 years ago. Updated over 3 years ago.

[ruby-dev:40961]
Status:Closed
Priority:Normal
Assignee:Keiju Ishitsuka
ruby -v:- Backport:

Description

=begin
いしつかさん
遠藤です。

にて Brian Ford が「complex を require すると
Math.atan(nil) で投げられる例外が変わる」という報告をしています。

$ ./ruby -e 'p Math.atanh(nil)'
-e:1:in atanh': can't convert nil into Float (TypeError)
from -e:1:in
'

$ ./ruby -rcomplex -e 'p Math.atanh(nil)'
/home/mame/work/ruby-trunk-local/lib/ruby/1.9.1/cmath.rb:196:in atanh': undefined methodreal?' for nil:NilClass (NoMethodError)
from -e:1:in `'

Ruby レベルのライブラリは duck typing のためにむやみに型チェック
すべきでないとはいえ、CMath は組み込みの Math クラスの置き換えを
前提としているので、なるべく Math クラスの挙動を尊重した方がよい
と思いました。

以下のパッチをコミットしてもいいでしょうか。

# ついでですが、 も見てください。

diff --git a/lib/cmath.rb b/lib/cmath.rb
index b23dac2..aa2d9bb 100644
--- a/lib/cmath.rb
+++ b/lib/cmath.rb
@@ -27,6 +27,7 @@ module CMath
alias atanh! atanh

def exp(z)
  • z = Float(z)
    if z.real?
    exp!(z)
    else
    @@ -36,9 +37,9 @@ module CMath
    end
    end

  • def log(*args)

  • z, b = args

  • if z.real? and z >= 0 and (b.nil? or b >= 0)

  • def log(z, b = nil)

  • z = Float(z)

  • if z.real? and z >= 0 and (b.nil? or (b = Float(b); b >= 0))
    log!(*args)
    else
    a = Complex(log!(z.abs), z.arg)
    @@ -50,6 +51,7 @@ module CMath
    end

    def log2(z)

  • z = Float(z)
    if z.real? and z >= 0
    log2!(z)
    else
    @@ -58,6 +60,7 @@ module CMath
    end

    def log10(z)

  • z = Float(z)
    if z.real? and z >= 0
    log10!(z)
    else
    @@ -66,6 +69,7 @@ module CMath
    end

    def sqrt(z)

  • z = Float(z)
    if z.real?
    if z < 0
    Complex(0, sqrt!(-z))
    @@ -85,6 +89,7 @@ module CMath
    end

    def cbrt(z)

  • z = Float(z)
    if z.real? and z >= 0
    cbrt!(z)
    else
    @@ -93,6 +98,7 @@ module CMath
    end

    def sin(z)

  • z = Float(z)
    if z.real?
    sin!(z)
    else
    @@ -102,6 +108,7 @@ module CMath
    end

    def cos(z)

  • z = Float(z)
    if z.real?
    cos!(z)
    else
    @@ -111,6 +118,7 @@ module CMath
    end

    def tan(z)

  • z = Float(z)
    if z.real?
    tan!(z)
    else
    @@ -119,6 +127,7 @@ module CMath
    end

    def sinh(z)

  • z = Float(z)
    if z.real?
    sinh!(z)
    else
    @@ -128,6 +137,7 @@ module CMath
    end

    def cosh(z)

  • z = Float(z)
    if z.real?
    cosh!(z)
    else
    @@ -137,6 +147,7 @@ module CMath
    end

    def tanh(z)

  • z = Float(z)
    if z.real?
    tanh!(z)
    else
    @@ -145,6 +156,7 @@ module CMath
    end

    def asin(z)

  • z = Float(z)
    if z.real? and z >= -1 and z <= 1
    asin!(z)
    else
    @@ -153,6 +165,7 @@ module CMath
    end

    def acos(z)

  • z = Float(z)
    if z.real? and z >= -1 and z <= 1
    acos!(z)
    else
    @@ -161,6 +174,7 @@ module CMath
    end

    def atan(z)

  • z = Float(z)
    if z.real?
    atan!(z)
    else
    @@ -169,6 +183,7 @@ module CMath
    end

    def atan2(y,x)

  • x, y = Float(x), Float(y)
    if y.real? and x.real?
    atan2!(y,x)
    else
    @@ -177,6 +192,7 @@ module CMath
    end

    def asinh(z)

  • z = Float(z)
    if z.real?
    asinh!(z)
    else
    @@ -185,6 +201,7 @@ module CMath
    end

    def acosh(z)

  • z = Float(z)
    if z.real? and z >= 1
    acosh!(z)
    else
    @@ -193,6 +210,7 @@ module CMath
    end

    def atanh(z)

  • z = Float(z)
    if z.real? and z >= -1 and z <= 1
    atanh!(z)
    else

    Yusuke Endoh mame@tsg.ne.jp
    =end


Related issues

Duplicates Ruby trunk - Bug #2756: Issues with Math and Complex behavior on 1.9 Assigned 02/18/2010

Associated revisions

Revision 32055
Added by Keiju Ishitsuka over 3 years ago

  • lib/cmath.rb: add new methd Object#real?. fix #3137

Revision 32055
Added by Keiju Ishitsuka over 3 years ago

  • lib/cmath.rb: add new methd Object#real?. fix #3137

Revision 32297
Added by Keiju Ishitsuka over 3 years ago

  • lib/cmath.rb: make same exception for Math. fix [Bug #3137].

Revision 32297
Added by Keiju Ishitsuka over 3 years ago

  • lib/cmath.rb: make same exception for Math. fix [Bug #3137].

History

#1 Updated by Marc-Andre Lafortune almost 5 years ago

=begin
A problem with the patch is that Float("1.1") returns 1.1.

In my todo list is "propose strict coercion methods"... I had to code them up in Matrix
=end

#2 Updated by Yusuke Endoh almost 5 years ago

  • Target version changed from 1.9.2 to 2.0.0

=begin
Hi,

2010年4月12日23:41 Marc-Andre Lafortune redmine@ruby-lang.org:

A problem with the patch is that Float("1.1") returns 1.1.

In my todo list is "propose strict coercion methods"... I had to code them up in Matrix

Oh, you are right. Thanks.

I guess "strict coercion" will be good, but it is absolutely new
feature. So, let's leave #2756 and this ticket to 1.9.3 (or later).

--
Yusuke ENDOH mame@tsg.ne.jp
=end

#3 Updated by Keiju Ishitsuka almost 5 years ago

=begin
けいじゅ@いしつかです.

In the message: "[Bug #3137] complex.rb changes exceptions of Math",
on Apr/12 23:01(JST) Yusuke Endoh writes:

遠藤です。

どもです.

にて Brian Ford が「complex を require すると
Math.atan(nil) で投げられる例外が変わる」という報告をしています。

$ ./ruby -e 'p Math.atanh(nil)'
-e:1:in atanh': can't convert nil into Float (TypeError)
from -e:1:in
'

$ ./ruby -rcomplex -e 'p Math.atanh(nil)'
/home/mame/work/ruby-trunk-local/lib/ruby/1.9.1/cmath.rb:196:in atanh': undefined methodreal?' for nil:NilClass (NoMethodError)
from -e:1:in `'

Ruby レベルのライブラリは duck typing のためにむやみに型チェック
すべきでないとはいえ、CMath は組み込みの Math クラスの置き換えを
前提としているので、なるべく Math クラスの挙動を尊重した方がよい
と思いました。

うーん. そうですねぇ...

ただ, Complex#atanh等で,

-e:1:in `atanh': can't convert nil into Float (TypeError)

という例外はふさわしくない気もします. Complex#atanhの実装を知らないと
わけの分からないエラーになっている気がしません?

Math.log(-1) の
Math::DomainError: Numerical argument is out of domain - "log"

ような例外の方がふさわしいかと?

以下のパッチをコミットしてもいいでしょうか。

添付のパッチだと zが複素数の時は動作しません.

例外はMathとは変わりますが, それでよいなら, NoMethodErrorを捕捉してそ
れなりの例外にして再raiseするようにします.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---
=end

#4 Updated by Keiju Ishitsuka almost 5 years ago

=begin
けいじゅ@いしつかです.

In the message: "[Bug #3137] complex.rb changes exceptions of Math",
on Apr/12 23:01(JST) Yusuke Endoh writes:

遠藤です。

どもです.

にて Brian Ford が「complex を require すると
Math.atan(nil) で投げられる例外が変わる」という報告をしています。

$ ./ruby -e 'p Math.atanh(nil)'
-e:1:in atanh': can't convert nil into Float (TypeError)
from -e:1:in
'

$ ./ruby -rcomplex -e 'p Math.atanh(nil)'
/home/mame/work/ruby-trunk-local/lib/ruby/1.9.1/cmath.rb:196:in atanh': undefined methodreal?' for nil:NilClass (NoMethodError)
from -e:1:in `'

Ruby レベルのライブラリは duck typing のためにむやみに型チェック
すべきでないとはいえ、CMath は組み込みの Math クラスの置き換えを
前提としているので、なるべく Math クラスの挙動を尊重した方がよい
と思いました。

うーん. そうですねぇ...

ただ, Complex#atanh等で,

-e:1:in `atanh': can't convert nil into Float (TypeError)

という例外はふさわしくない気もします. Complex#atanhの実装を知らないと
わけの分からないエラーになっている気がしません?

Math.log(-1) の
Math::DomainError: Numerical argument is out of domain - "log"

ような例外の方がふさわしいかと?

以下のパッチをコミットしてもいいでしょうか。

添付のパッチだと zが複素数の時は動作しません.

例外はMathとは変わりますが, それでよいなら, NoMethodErrorを捕捉してそ
れなりの例外にして再raiseするようにします.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

=end

#5 Updated by Yukihiro Matsumoto almost 5 years ago

=begin
まつもと ゆきひろです

In message "Re: Re: [Bug #3137] complex.rb changes exceptions of Math"
on Tue, 13 Apr 2010 14:56:59 +0900, keiju@ishitsuka.com (石塚圭樹) writes:

|うーん. そうですねぇ...
|
|ただ, Complex#atanh等で,
|
|> -e:1:in `atanh': can't convert nil into Float (TypeError)
|
|という例外はふさわしくない気もします. Complex#atanhの実装を知らないと
|わけの分からないエラーになっている気がしません?

そう? nil渡して「nilじゃダメ」と言われてるんだから明確でな
いかと。「Floatでなくてもいいじゃん」ってことなんだと思うけ
ど、そこは致命的ではないのでは。

=end

#6 Updated by Keiju Ishitsuka almost 5 years ago

=begin
けいじゅ@いしつかです.

In the message: " Re: [Bug #3137]
complex.rb changes exceptions of Math", on Apr/13 15:19(JST) Yukihiro
Matsumoto writes:

まつもと ゆきひろです

|ただ, Complex#atanh等で,
|> -e:1:in `atanh': can't convert nil into Float (TypeError)
|という例外はふさわしくない気もします. Complex#atanhの実装を知らないと
|わけの分からないエラーになっている気がしません?

そう? nil渡して「nilじゃダメ」と言われてるんだから明確でな
いかと。「Floatでなくてもいいじゃん」ってことなんだと思うけ
ど、そこは致命的ではないのでは。

実装案を見ると:

def exp(z)
begin
if z.real?
exp!(z)
else
ere = exp!(z.real)
Complex(ere * cos!(z.imag),
ere * sin!(z.imag))
end
rescue NoMethodError => exp
if exp.name == :real?
raise "error"
end
raise
end
end

こんな感じです. "error"のところの例外をどうするかって話で, ここで

|> -e:1:in `atanh': can't convert nil into Float (TypeError)

ちょっと分かりづらい例外かとおもったんですね.

あと,

def exp(z)
begin
if z.real?
exp!(z)
else
ere = exp!(z.real)
Complex(ere * cos!(z.imag),
ere * sin!(z.imag))
end
rescue NoMethodError => exp
if exp.name == :real?
return exp(Float(z))
end
raise
end
end

はあるかなぁ... Numericとそのサブクラスしか, real? って持ってませんの
で, それ以外のクラスはMathと同じ振る舞いということで, Floatに変換して
試すはありかも. これだと, CMathの担当範囲外はMathと同じ結果になります.
ちょっと, くどい気がしなくもありませんが...

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

=end

#7 Updated by Yukihiro Matsumoto almost 5 years ago

=begin
まつもと ゆきひろです

In message "Re: Re: [Bug #3137] complex.rb changes exceptions of Math"
on Tue, 13 Apr 2010 16:30:38 +0900, keiju@ishitsuka.com (石塚圭樹) writes:

|こんな感じです. "error"のところの例外をどうするかって話で, ここで
|
|>|> -e:1:in `atanh': can't convert nil into Float (TypeError)
|
|ちょっと分かりづらい例外かとおもったんですね.

そうかなあ。そのまま、「real?がない」でいいんじゃないですかね。

|あと,
|
| def exp(z)
| begin
| if z.real?
| exp!(z)
| else
| ere = exp!(z.real)
| Complex(ere * cos!(z.imag),
| ere * sin!(z.imag))
| end
| rescue NoMethodError => exp
| if exp.name == :real?
| return exp(Float(z))
| end
| raise
| end
| end
|
|はあるかなぁ...

これは exp("1.1") なんてのを受け付けるようになっちゃうんで賛
成しません。

=end

#8 Updated by Keiju Ishitsuka almost 5 years ago

=begin
けいじゅ@いしつかです.

In the message: " Re: [Bug #3137]
complex.rb changes exceptions of Math", on Apr/13 16:47(JST) Yukihiro
Matsumoto writes:

まつもと ゆきひろです

|こんな感じです. "error"のところの例外をどうするかって話で, ここで
|>|> -e:1:in `atanh': can't convert nil into Float (TypeError)
|ちょっと分かりづらい例外かとおもったんですね.
そうかなあ。そのまま、「real?がない」でいいんじゃないですかね。

それは, 現行の状態です. そうじゃなくて, Mathと同じ上記の例外にしろって
言われているんですが?

私も積極的に変えなくちゃって気もしているわけではないので, rejectでもか
まいませんけど...
起票者の遠藤さんはどうかな?

これは exp("1.1") なんてのを受け付けるようになっちゃうんで賛
成しません。

あー. まちがいです

def exp(z)
begin
if z.real?
exp!(z)
else
ere = exp!(z.real)
Complex(ere * cos!(z.imag),
ere * sin!(z.imag))
end
rescue NoMethodError => exp
if exp.name == :real?
return exp!(z)
end
raise
end
end

でした.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

=end

#9 Updated by Yukihiro Matsumoto almost 5 years ago

=begin
まつもと ゆきひろです

In message "Re: Re: [Bug #3137] complex.rb changes exceptions of Math"
on Tue, 13 Apr 2010 17:05:17 +0900, keiju@ishitsuka.com (石塚圭樹) writes:

|それは, 現行の状態です. そうじゃなくて, Mathと同じ上記の例外にしろって
|言われているんですが?
|
|私も積極的に変えなくちゃって気もしているわけではないので, rejectでもか
|まいませんけど...
|起票者の遠藤さんはどうかな?

うーん、私は中立です。まあ、Floatをかませずにオリジナルのexp
を呼ぶぶんには実害は少ないのかな。

=end

#10 Updated by Yusuke Endoh almost 5 years ago

=begin
遠藤です。

2010年4月13日17:11 Yukihiro Matsumoto matz@ruby-lang.org:

まつもと ゆきひろです

In message "Re: Re: [Bug #3137] complex.rb changes exceptions of Math"
on Tue, 13 Apr 2010 17:05:17 +0900, keiju@ishitsuka.com (石塚圭樹) writes:

|それは, 現行の状態です. そうじゃなくて, Mathと同じ上記の例外にしろって
|言われているんですが?
|
|私も積極的に変えなくちゃって気もしているわけではないので, rejectでもか
|まいませんけど...
|起票者の遠藤さんはどうかな?

うーん、私は中立です。まあ、Floatをかませずにオリジナルのexp
を呼ぶぶんには実害は少ないのかな。

エラーメッセージの問題ではなく、例外クラスが変わることだと思い
ます。

def tolerant_exp(x)
begin
Math.exp(x)
rescue TypeError
0
end
end

というコードの挙動が、complex.rb をロードするかどうかで変わって
しまうという……。こんな例がどのくらい存在するのか知りませんが。

# 前も言ったけど、NoMethodError が TypeError を継承してないのは
# 設計ミスだと思う

個人的には、何が何でも直すべきとは思ってませんので、reject でも
構いません。その場合は、 の Brian Ford に返事を
してあげてもらえると助かります。

--
Yusuke ENDOH mame@tsg.ne.jp

=end

#11 Updated by Keiju Ishitsuka almost 5 years ago

=begin
けいじゅ@いしつかです.

In the message: " Re: [Bug #3137]
complex.rb changes exceptions of Math", on Apr/13 22:42(JST) Yusuke
ENDOH writes:

遠藤です。

エラーメッセージの問題ではなく、例外クラスが変わることだと思い
ます。

def tolerant_exp(x)
begin
Math.exp(x)
rescue TypeError
0
end
end

というコードの挙動が、complex.rb をロードするかどうかで変わって
しまうという……。こんな例がどのくらい存在するのか知りませんが。

うーん. 言いたいことは分からなくはないですが...

個人的には、何が何でも直すべきとは思ってませんので、reject でも
構いません。その場合は、 の Brian Ford に返事を
してあげてもらえると助かります。

見ました. 同じ件([Bug#1788]で船場さんが以前にrejectしていたんです
ね. 遠藤さんに直接呼び出されたので, 無条件反射的に出てきてしまいました
が, cmath.rb は, 船場さんの担当がふさわしいと認識しています.

というわけで, どうするかは船場さんにお願いしたいのですが?

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

=end

#12 Updated by Yusuke Endoh almost 5 years ago

=begin
遠藤です。

2010年4月14日13:32 石塚圭樹 keiju@ishitsuka.com:

個人的には、何が何でも直すべきとは思ってませんので、reject でも
構いません。その場合は、 の Brian Ford に返事を
してあげてもらえると助かります。

見ました. 同じ件([Bug#1788]で船場さんが以前にrejectしていたんです
ね. 遠藤さんに直接呼び出されたので, 無条件反射的に出てきてしまいました
が, cmath.rb は, 船場さんの担当がふさわしいと認識しています.

えっ、あれっ、cmath.rb のメンテナはいしつかさんだと思っていた
のですが、メンテナ一覧には全然そんなこと書いてないですね。
完全に勘違いしていました。失礼しました。

というわけで, どうするかは船場さんにお願いしたいのですが?

ふなばさんがメンテナをやりますか?

メンテナがいないのであれば、Marc-Andre がそのうち引き取って
くれそうな気がするので、それでもいいかなと思いました。

--
Yusuke ENDOH mame@tsg.ne.jp

=end

#13 Updated by tadayoshi funaba almost 5 years ago

=begin
lib/cmath.rb は担当しないといけないかと思っていたのですが、
この前の確認で違うという事が判明したので以後は関与してませんでした。

cmath.rb (というか、以前からの complex.rb の数学関数) は他にも課題がありそうなんで、
当初は Math と統合するか少なくとも C で書き直して、その際にいろいろと改善できれば
いいと思っていましたが、そういう機会はなかったし、今後もなさそうだし、
そもそも、rational.c や complex.c に担当はない状態で、
旧い complex.rb からの遺産 (どちらかというとマイナスの) だけ担当する意味は極めて薄いわけで、
他にやりたい人が居るならやってもらっていいと思います。

=end

#14 Updated by Shyouhei Urabe over 4 years ago

  • Status changed from Open to Assigned

=begin

=end

#15 Updated by Koichi Sasada over 3 years ago

この件,どうしましょうか.

#16 Updated by tadayoshi funaba over 3 years ago

  • ruby -v changed from ruby 1.9.2dev (2010-04-12 trunk 27317) [i686-linux] to -

それであれば, CMathのなかで, Object#real? を定義して, TypeErrorを出す
ようにしようと思います.

単純に疑問ですが、この方法は良いのでしょうか?

個別に Numeric であるか確認するか、どうしてもこのやり方をするのであれば、
complex.rb でやるとか。complex.rb をつかわなければ、Math は置き換えられ
ませんから。

#17 Updated by tadayoshi funaba over 3 years ago

それであれば, CMathのなかで, Object#real? を定義して, TypeErrorを出す
ようにしようと思います.

単純に疑問ですが、この方法は良いのでしょうか?

個別に Numeric であるか確認するか、どうしてもこのやり方をするのであれば、
complex.rb でやるとか。complex.rb をつかわなければ、Math は置き換えられ
ませんから。

#18 Updated by Keiju Ishitsuka over 3 years ago

けいじゅ@いしつかです.

In the message: " Re: [Ruby 1.9 - Bug
#3137] complex.rb changes exceptions of Math", on Jun/14 00:25(JST)
Tadayoshi Funaba writes:

それであれば, CMathのなかで, Object#real? を定義して, TypeErrorを出す
ようにしようと思います.

単純に疑問ですが、この方法は良いのでしょうか?

グローバルなメソッドを定義し, かつ名前が悪いってことです?

個別に Numeric であるか確認するか、どうしてもこのやり方をするのであれば、
complex.rb でやるとか。complex.rb をつかわなければ、Math は置き換えられ
ませんから。

後者は, MathとCMathで違う例外になってしまいますし, 例外の種類があまり
ふさわしくないのも事実なので, 採用するなら前者になると思いますが.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

#19 Updated by Keiju Ishitsuka over 3 years ago

けいじゅ@いしつかです.

In the message: " Re: [Ruby 1.9 - Bug
#3137] complex.rb changes exceptions of Math", on Jun/14 00:25(JST)
Tadayoshi Funaba writes:

それであれば, CMathのなかで, Object#real? を定義して, TypeErrorを出す
ようにしようと思います.

単純に疑問ですが、この方法は良いのでしょうか?

グローバルなメソッドを定義し, かつ名前が悪いってことです?

個別に Numeric であるか確認するか、どうしてもこのやり方をするのであれば、
complex.rb でやるとか。complex.rb をつかわなければ、Math は置き換えられ
ませんから。

後者は, MathとCMathで違う例外になってしまいますし, 例外の種類があまり
ふさわしくないのも事実なので, 採用するなら前者になると思いますが.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

#20 Updated by tadayoshi funaba over 3 years ago

グローバルなメソッドを定義し, かつ名前が悪いってことです?

俺も Object とか Numeric にごっそりメソッドを確保しておこうかなあ、他所
で取られる前に。

#21 Updated by tadayoshi funaba over 3 years ago

グローバルなメソッドを定義し, かつ名前が悪いってことです?

俺も Object とか Numeric にごっそりメソッドを確保しておこうかなあ、他所
で取られる前に。

#22 Updated by Sho Hashimoto over 3 years ago

r32055 の修正だと、CMath.#log で底に数値以外を与える場合に Math.#log のように TypeError が発生しませんでした。

CMath.log(8, "2") # => ArgumentError

#23 Updated by Keiju Ishitsuka over 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r32297.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • lib/cmath.rb: make same exception for Math. fix [Bug #3137].

Also available in: Atom PDF