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 lift_centered for more classes #15804

Closed
lftabera opened this issue Feb 10, 2014 · 31 comments
Closed

Add lift_centered for more classes #15804

lftabera opened this issue Feb 10, 2014 · 31 comments

Comments

@lftabera
Copy link
Contributor

This ticket is a split of #8558 for easier review and merging.

Add a lift_centered method to ntl_mod_p elements and a generic function.

If p is an integer mod n, returns r congruent to p mod n such that -n/2 < r <= n/2

It also deprecates the use of centerlift for lift_centered. The discussion is in #8558

Component: basic arithmetic

Keywords: modular arithmetic, centered lift

Author: Luis Felipe Tabera Alonso, Jeroen Demeyer

Branch/Commit: c2255e5

Reviewer: Vincent Delecroix

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

@lftabera lftabera added this to the sage-6.2 milestone Feb 10, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 10, 2014

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

e6bd63bDeprecate centerlift method

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 10, 2014

Changed commit from ddb7291 to e6bd63b

@jdemeyer
Copy link

comment:3

I would write the global function lift_centered as

try:
    return p.lift_centered()
except AttributeError:
    return p.lift()

such that it always works on modular objects, even if .lift_centered() has no meaning.

@lftabera
Copy link
Contributor Author

comment:4

Are you thinking for instance?

K.<x,y,z> = QQ[]
I = Ideal(x*y,x*z,y*z)
R = K.quotient_ring(I)
f = R((x+y+z)**2)
lift_centered(f)
x^2 + y^2 + z^2

So what you propose is that lift_centered should be always a class method and never explicitely computed using the global function, don't you?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

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

169cb08Make global lift_centered function fallback to lift if the object

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2014

Changed commit from e6bd63b to 169cb08

@jdemeyer
Copy link

comment:6

In Python, it is better not to use hasattr. Instead, catch AttributeError.

Also, the normal syntax is except AttributeError: (without parentheses).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2014

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

868e571Catch exceptions instead of using hassattr.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2014

Changed commit from 169cb08 to 868e571

@lftabera
Copy link
Contributor Author

comment:8

Fixed, I have also changed the documentation, I hope it is in better shape now.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2014

Changed commit from 868e571 to 406dca9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2014

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

9da4eabMerge branch 'master' into u/lftabera/lift_centered
406dca9Fix import error

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 16, 2014

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

e81e7a0Merge branch 'master' into u/lftabera/lift_centered

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 16, 2014

Changed commit from 406dca9 to e81e7a0

@jdemeyer
Copy link

comment:13

For the newly added docstrings, you should follow the format explained in http://www.sagemath.org/doc/developer/coding_basics.html#documentation-strings. Note that the INPUT and OUTPUT blocks should not be indented and that the docstring shouldn't start with an empty line.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 1, 2015

Changed commit from 3dd232b to 584e874

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 1, 2015

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

a53a956Merge tag '6.10.beta6' into u/lftabera/lift_centered
584e874Trac #15804: Add lift_centered for more classes

@lftabera
Copy link
Contributor Author

lftabera commented Dec 3, 2015

comment:19

Ups, sorry, the docstring format was not good. I have fixed it.

@videlec: After thinking about it, the global namespace is already too big. It should be used for essential functions only. And preferably those that are "classical" or have sense for an arbitrary Sage element. So, I have deleted the global function, the user should depend on the method of each class.

@videlec
Copy link
Contributor

videlec commented Dec 4, 2015

comment:20

Couldn't you do some economy in pari.gen

cdef class gen(gen_auto):
   ...
   lift_centered = centerlift

@jdemeyer
Copy link

comment:21

Replying to @videlec:

Couldn't you do some economy in pari.gen

cdef class gen(gen_auto):
   ...
   lift_centered = centerlift

I agree.

@jdemeyer
Copy link

Changed branch from u/lftabera/lift_centered to u/jdemeyer/lift_centered

@jdemeyer
Copy link

New commits:

c2255e5For PARI, define lift_centered as alias of centerlift

@jdemeyer
Copy link

Changed author from Luis Felipe Tabera Alonso to Luis Felipe Tabera Alonso, Jeroen Demeyer

@jdemeyer
Copy link

Changed commit from 584e874 to c2255e5

@videlec
Copy link
Contributor

videlec commented Dec 24, 2015

comment:24

This is good for me.

@jdemeyer
Copy link

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Dec 25, 2015

Changed branch from u/jdemeyer/lift_centered to c2255e5

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

4 participants