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

py3: future division in matrix2 #23731

Closed
fchapoton opened this issue Aug 27, 2017 · 14 comments
Closed

py3: future division in matrix2 #23731

fchapoton opened this issue Aug 27, 2017 · 14 comments

Comments

@fchapoton
Copy link
Contributor

part of #15995

found using the "python -3" tests suggested there

Component: python3

Keywords: division

Author: Frédéric Chapoton

Branch: fdf0a68

Reviewer: André Apitzsch, Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/23731

@fchapoton fchapoton added this to the sage-8.1 milestone Aug 27, 2017
@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/23731

@fchapoton
Copy link
Contributor Author

Commit: e65a63e

@fchapoton
Copy link
Contributor Author

New commits:

e65a63epy3: future division in matrix2

@fchapoton fchapoton changed the title py3: future division in matrix 2 py3: future division in matrix2 Aug 27, 2017
@a-andre
Copy link

a-andre commented Aug 27, 2017

Reviewer: André Apitzsch

@a-andre
Copy link

a-andre commented Aug 27, 2017

comment:3

LGTM.

@jdemeyer
Copy link

comment:4

I disagree here: why hard-code QQ? Shouldn't we use self._parent._base instead?

@@ -1371,7 +1372,7 @@ cdef class Matrix(Matrix1):
         # now compute the permanental minor of the complement matrix if needed
         if complement:
             a = [one]
-            c1 = 1
+            c1 = QQ.one()
             for k in range(1, mn + 1):
                 c1 = c1*(m-k+1)*(n-k+1)/k
                 c = c1

@fchapoton
Copy link
Contributor Author

comment:5

Because for this precise variable, we can stay in QQ. Otherwise, more complicated computations are done.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2017

Changed commit from e65a63e to fdf0a68

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

fdf0a68trac 23731 try exact division

@fchapoton
Copy link
Contributor Author

comment:7

So, what about the new commit ? The first // makes sense (binomial coefficients) but i would not swear that the second one does make sense too.


New commits:

fdf0a68trac 23731 try exact division

@jdemeyer
Copy link

Changed reviewer from André Apitzsch to André Apitzsch, Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Sep 10, 2017

Changed branch from u/chapoton/23731 to fdf0a68

@embray
Copy link
Contributor

embray commented Feb 23, 2018

Changed commit from fdf0a68 to none

@embray
Copy link
Contributor

embray commented Feb 23, 2018

Changed keywords from none to division

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants