Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Matrix#antisymmetric? and Matrix#reflexive? #1730

Closed
wants to merge 14 commits into from
Closed

Add Matrix#antisymmetric? and Matrix#reflexive? #1730

wants to merge 14 commits into from

Conversation

fang-niuwa
Copy link

Add two methods (“antisymmetric?” and “reflexive?”) determine if the
matrix is reflexive, antisymmetric or not.

Add two methods (“antisymmetric?” and “reflexive?”) determine if the
matrix is reflexive, antisymmetric or not.
@marcandre
Copy link
Member

Thanks for the PR.
These look like valid additions to the library.
The PR some issues though:

  • needs specs, ideally in `ruby/spec/library/matrix/
  • the code for antisymmetric looks wrong
  • would probably be nicer to use all? instead of the return true/false

Would you like to amend your PR?

@marcandre marcandre changed the title Add two methods. Add two methods to the Matrix library. Dec 14, 2017
@marcandre marcandre changed the title Add two methods to the Matrix library. Add Matrix#antisymmetric? and Matrix#reflexive? Dec 14, 2017
Yilo added 3 commits December 25, 2017 12:41
1. implement method — antisymmetric?
2. use all? instead of the return true/false
3. add specs for both methods
lib/matrix.rb Outdated
@@ -28,6 +28,7 @@ module ExceptionForMatrix # :nodoc:
#
# The +Matrix+ class represents a mathematical matrix. It provides methods for creating
# matrices, operating on them arithmetically and algebraically,
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops


describe "Matrix.antisymmetric?" do
it "returns true for an antisymmetric matrix" do
Matrix[[1, 2, 3], [4, 5, 6], [7, 8, 9]].antisymmetric?.should be_true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not an antisymmetric matrix

end

it "returns true for a 0x0 empty matrix" do
Matrix.empty.symmetric?should be_true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong method

Matrix.empty.symmetric?should be_true
end

it "returns false for an asymmetric Matrix" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??? (An asymmetric matrix can be antisymmetric; what‘s the point of this test?)

end

it "returns false for an asymmetric Matrix" do
Matrix[[1, 2],[-2, 1]].symmetric?.should be_false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong method

Matrix[[1, 2],[-2, 1]].symmetric?.should be_false
end

it "raises an error for rectangular matrices" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO clearer: non-square matrices

# Returns +true+ if this is a antisymmetric matrix.
# Raises an error if matrix is not square.
#
def antisymmetric?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation does not test for antisymmetric!

end

it "returns true for a 0x0 empty matrix" do
Matrix.empty.symmetric?.should be_true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong method

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for being so patient. Most of the code were follow the style of method "symmetric?". I've learned 1-0 matrix, and it seems to me that reflexive relation cannot applied in a general matrix, this method might not be very helpful, but I'll still fix this problem.

Matrix.empty.symmetric?.should be_true
end

it "returns true for a non-reflexive Matrix" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true?

Matrix[[1, 1],[2, 2]].reflexive?.should be_false
end

it "raises an error for rectangular matrices" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-square

Matrix.empty(2, 0),
].each do |rectangular_matrix|
lambda {
rectangular_matrix.symmetric?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong method

@stomar
Copy link
Contributor

stomar commented Dec 27, 2017

and: antisymmetric_spec.rb is not in the correct directory

Copy link
Member

@marcandre marcandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

antisymetric? has already been added, please remove it and keep only reflexive?

#
def reflexive?
Matrix.Raise ErrDimensionMismatch unless square?
self.each(:diagonal) do |e|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self is superfluous. consider using each(:diagonal).all?

self.each(:diagonal) do |e|
return false if e != 1
end
all?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect.

@marcandre
Copy link
Member

Implementation fixed, code merged 😄

Thanks @Yiloo!

@marcandre marcandre closed this Sep 20, 2018
matzbot pushed a commit that referenced this pull request Sep 20, 2018
Adapted from a patch by Yilo

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64796 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
znz added a commit to znz/doctree that referenced this pull request Sep 21, 2018
@mame
Copy link
Member

mame commented Sep 21, 2018

Is "reflexive" a common word to indicate a matrix whose diagonals are all 1s?

I couldn't (easily) find such a usage by searching with "reflexive matrix" diagonal. I could find some articles including the definition by searching with "matrix is reflexive", but its hit count is just 44. I guess that the word came from "reflexive relation", but I'm unsure if it is also common for matrix.

@marcandre What do you think? /cc @znz

@marcandre
Copy link
Member

Hmmm, it looks like indeed it's not needed (and easy to do anyways). I'll revert this.

@mame
Copy link
Member

mame commented Sep 21, 2018

/cc @stomar @marcandre

Well, there seems to be confusion about the word "antisymmetric".

I believe the OP expected antisymmetric relation: a_ij = 0 or a_ji = 0 for all i ≠ j. However, the PR's implementation checks if a_ij != a_ji for all i ≠ j. I guess it was a bug.
Matrix#antisymmetric in @stomar's #1788, which is the current implementation in trunk, checks if the matrix is skew-symmetric: a_ij == -a_ji for all i != j.
I think that antisymmetric relation is not directly related to antisymmetric matrix. Anyway, the current implementation is different than the OP expected.

Wikipedia's "Skew-symmetric matrix" says (emphasized by me):

In mathematics, particularly in linear algebra, a skew-symmetric (or antisymmetric or antimetric[1]) matrix is a square matrix whose transpose equals its negative

If we have Matrix#antisymmetric?, it should use this definition of "antisymmetric matrix", not "antisymmetric relation". So, @stomar's implementation is the best, I think.

However, I'm not sure whether the name "antisymmetric?" is best. Isn't "skew_symmetric?" better? I'm unfamiliar with math. Wolfram uses the name AntisymmetricMatrix, but it also says:

Antisymmetric matrices are commonly called "skew symmetric matrices" by mathematicians.

Strictly speaking, there is no actual request for the current Matrix#antisymmetric?. Is it really needed? Again, I'm unfamiliar with math.

@mame
Copy link
Member

mame commented Sep 21, 2018

@mrkn found: https://physics.stackexchange.com/questions/96687/what-is-the-difference-between-a-skew-symmetric-and-an-antisymmetric-tensor

Physicists tend to favor the term "antisymmetric" and mathematicians tend to favor the term "skew-symmetric" in my experience.

@marcandre
Copy link
Member

Strictly speaking, there is no actual request for the current Matrix#antisymmetric?. Is it really needed? Again, I'm unfamiliar with math.

I think that antisymmetric? is well defined and thus could be useful to many (contrary to reflexive?). I could add an alias for skew_symmetric? though.

@stomar
Copy link
Contributor

stomar commented Sep 29, 2018

Actually, I am a physicist 😄. I thought the concept useful and used enough to have it added to Matrix. An alias might be a good idea.

@fang-niuwa
Copy link
Author

Is "reflexive" a common word to indicate a matrix whose diagonals are all 1s?

I couldn't (easily) find such a usage by searching with "reflexive matrix" diagonal. I could find some articles including the definition by searching with "matrix is reflexive", but its hit count is just 44. I guess that the word came from "reflexive relation", but I'm unsure if it is also common for matrix.

@marcandre What do you think? /cc @znz

It was my idea to implement this method, as I was taking Discrete Mathematics course in the college. You are right, the reflexive matrix is representing a reflexive relation by using 0-1 matrix. I was not sure if this method would be widely used since a general matrix can have more values than just 0 and 1.

@mame
Copy link
Member

mame commented Nov 1, 2018

Thank you for your reply. Now I'm satisfied about antisymmetric?.

@marcandre May I remove reflexive?? I'd like to remove it before 2.6.0 preview 3 if possible.

@marcandre
Copy link
Member

marcandre commented Nov 1, 2018

I'll do it tomorrow, and merge the mutable matrix too, assuming that's ok for you, @mame?

marcandre added a commit to marcandre/ruby that referenced this pull request Nov 2, 2018
matzbot pushed a commit that referenced this pull request Nov 2, 2018
This reverts commit 19fe655.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65500 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants