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

Improve creation time for p-adic elements #9814

Closed
roed314 opened this issue Aug 27, 2010 · 12 comments
Closed

Improve creation time for p-adic elements #9814

roed314 opened this issue Aug 27, 2010 · 12 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Aug 27, 2010

I've implemented coercion morphisms from ZZ and QQ to Zp and Qp. This drops item creation time from about 20 microseconds to about 1 microsecond on my machine.

Component: padics

Author: David Roe

Reviewer: David Harvey

Merged: sage-4.6.alpha2

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

@roed314
Copy link
Contributor Author

roed314 commented Aug 27, 2010

@sagetrac-dmharvey
Copy link
Mannequin

sagetrac-dmharvey mannequin commented Aug 28, 2010

comment:2

seems to work as advertised, but I get lots of doctest failures with -testall, typical example is like this:

File "/Users/david/sage-4.5.2/devel/sage/sage/rings/padics/padic_generic_element.pyx", line 1002:
    sage: R3(-1).square_root() == R3.teichmuller(2) or R3(-1).square_root() == R3.teichmuller(3)
Exception raised:
    Traceback (most recent call last):
      File "/Users/david/sage-4.5.2/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/Users/david/sage-4.5.2/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/Users/david/sage-4.5.2/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_24[33]>", line 1, in <module>
        R3(-Integer(1)).square_root() == R3.teichmuller(Integer(2)) or R3(-Integer(1)).square_root() == R3.teichmuller(Integer(3))###line 1002:
    sage: R3(-1).square_root() == R3.teichmuller(2) or R3(-1).square_root() == R3.teichmuller(3)
      File "/Users/david/sage-4.5.2/local/lib/python/site-packages/sage/rings/padics/padic_generic.py", line 377, in teichmuller
        ans = self(x, prec)
      File "parent.pyx", line 861, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6427)
      File "map.pyx", line 478, in sage.categories.map.Map._call_with_args (sage/categories/map.c:3666)
    NotImplementedError: _call_with_args not overridden to accept arguments for <type 'sage.rings.padics.padic_base_coercion.pAdicCoercion_ZZ_CR'>

@aghitza
Copy link

aghitza commented Sep 2, 2010

Changed author from roed to David Roe

@roed314
Copy link
Contributor Author

roed314 commented Sep 18, 2010

Replaces first patch.

@roed314
Copy link
Contributor Author

roed314 commented Sep 18, 2010

comment:4

Attachment: trac_9814-2.patch.gz

Well, that was more work than I expected, but it also fixes some problems with non-uniqueness of p-adic parents.

@sagetrac-dmharvey
Copy link
Mannequin

sagetrac-dmharvey mannequin commented Sep 22, 2010

comment:5

Excellent. You've restored my faith in Sage :-)

@roed314
Copy link
Contributor Author

roed314 commented Sep 22, 2010

comment:6

Good. The series of patches in 7883 -> 8333 -> 8334 plus a bit of other additional work will fix most of your object creation problems with IntegerMods as well.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 22, 2010

Reviewer: David Harvey

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 28, 2010

comment:8

Could someone prepend the ticket number to the commit string's first line and restore the positive review? The first line should also be < 80 characters long, so that hg log messages are compact.

@qed777 qed777 mannequin added s: needs work and removed s: positive review labels Sep 28, 2010
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 28, 2010

Attachment: trac_9814-3.patch.gz

Commit message fixed. Apply only this patch.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 28, 2010

comment:9

Done.

@loefflerd loefflerd mannequin removed the s: needs work label Sep 28, 2010
@loefflerd loefflerd mannequin added the s: positive review label Sep 28, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 28, 2010

Merged: sage-4.6.alpha2

@qed777 qed777 mannequin removed the s: positive review label Sep 28, 2010
@qed777 qed777 mannequin closed this as completed Sep 28, 2010
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

2 participants