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

move integer ring to the new coercion model #4058

Closed
robertwb opened this issue Sep 4, 2008 · 12 comments
Closed

move integer ring to the new coercion model #4058

robertwb opened this issue Sep 4, 2008 · 12 comments

Comments

@robertwb
Copy link
Contributor

robertwb commented Sep 4, 2008

A couple of bugfixes are included as well.

CC: @aghitza @malb

Component: coercion

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

@robertwb robertwb added this to the sage-3.1.3 milestone Sep 4, 2008
@robertwb robertwb self-assigned this Sep 4, 2008
@robertwb
Copy link
Contributor Author

robertwb commented Sep 4, 2008

Attachment: 4058-integer-coerce.patch.gz

@aghitza
Copy link

aghitza commented Sep 20, 2008

comment:1

I'm getting an error trying to apply this patch to a fresh 3.1.2:

patching file sage/interfaces/expect.py
Hunk #1 FAILED at 1385
1 out of 1 hunk FAILED -- saving rejects to file sage/interfaces/expect.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

@robertwb
Copy link
Contributor Author

comment:2

I'll rebase this as soon as I get 3.1.2.

@robertwb
Copy link
Contributor Author

Attachment: 4058-integer-coerce.2.patch.gz

@robertwb
Copy link
Contributor Author

comment:4

Refreshed the patch so it applies cleanly to 3.1.2.

@mwhansen
Copy link
Contributor

comment:5

Looks good to me.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 24, 2008

comment:6

One thing I notice with this patch is that sr.py now takes around 650 seconds instead of 450 or so:

sage -t -long devel/sage/sage/crypto/mq/sr.py
         [655.1 s]

I am still merging the patch, but can we get this issue fixed next?

Cheers,

Michael

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 24, 2008

comment:7

Merged 4058-integer-coerce.2.patch in Sage 3.1.3.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Sep 24, 2008
@robertwb
Copy link
Contributor Author

comment:8

I will certainly look into that.

@robertwb
Copy link
Contributor Author

comment:9

Interestingly enough, on my machine sage -t sage/crypto/mq/sr.py, so it is just the (single) long doctest that provides the slowdown.

@robertwb
Copy link
Contributor Author

comment:10

See #4186 for a fix.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 24, 2008

comment:11

Replying to @robertwb:

Interestingly enough, on my machine sage -t sage/crypto/mq/sr.py, so it is just the (single) long doctest that provides the slowdown.

Yeah, in that doctest we do some wacky coercion into some mv polynomial ring with a couple thousand variables, so this is really an interesting test case. This was first discussed at SD6 in Bristol, so it just shows how much the coercion re-re-write has been an interesting road :)

Cheers,

Michael

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

3 participants