Opened 9 years ago

Closed 9 years ago

#11781 closed defect (fixed)

Conversion from Zmod element to padic element raises "not a power of the same prime" incorrectly, add conversion from residue_field

Reported by: robharron Owned by: roed
Priority: minor Milestone: sage-4.8
Component: padics Keywords: padics, conversion
Cc: roed Merged in: sage-4.8.alpha2
Authors: Robert Harron Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Since mpz_cmp_ui returns 0 if equality holds, wrong 'if' condition was being used to check if Zmod element was in the right characteristic. Fix this in padic_ZZ_pX_CR_element.pyx and padic_ZZ_pX_CA_element.pyx.

Also, enhancement: allow conversion from element of the residue_field of self (this really only affects unramified extensions).

Apply trac_11781_padics_conversion_Zmod_fix_and_res_field_enhance.v2.patch

Attachments (2)

trac_11781_padics_conversion_Zmod_fix_and_res_field_enhance.patch (6.1 KB) - added by robharron 9 years ago.
In padics, fix bug with Zmod element conversion, add conversion from residue_field
trac_11781_padics_conversion_Zmod_fix_and_res_field_enhance.v2.patch (5.9 KB) - added by jdemeyer 9 years ago.
Fix duplicate header

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by robharron

  • Status changed from new to needs_review

comment:2 follow-up: Changed 9 years ago by roed

  • Status changed from needs_review to needs_work

There seems to be a problem with the patch you uploaded: it just consists of the header. How did you create it?

comment:3 in reply to: ↑ 2 Changed 9 years ago by robharron

I followed the instructions at http://www.sagemath.org/doc/developer/walk_through.html under the section "Creating your own patches with queues", but I'm new to this whole thing, so I may have messed something up. Is there something specific I should do because these are cython files? I ran an hg qnew, then I edited, then I ran an hg qrefresh, then I ran an hg export tip. The sage build knows that I have updated them, but it's true that when I run hg qdiff it returns nothing. I'll try recreating the patch using the hg_sage interface, but I'll have to reclone first.

Replying to roed:

There seems to be a problem with the patch you uploaded: it just consists of the header. How did you create it?

comment:4 follow-up: Changed 9 years ago by roed

Try running "hg export qtip" rather than "hg export tip."

comment:5 in reply to: ↑ 4 ; follow-up: Changed 9 years ago by roed

Replying to roed:

Try running "hg export qtip" rather than "hg export tip."

You should be able to do this on your existing branch. You can then look at the patch (either before or after you upload it) in order to check to see if it includes the changes you want.

Thanks for working on this!

comment:6 in reply to: ↑ 5 Changed 9 years ago by robharron

qtip only produced the header as well.

Replying to roed:

Replying to roed:

Try running "hg export qtip" rather than "hg export tip."

You should be able to do this on your existing branch. You can then look at the patch (either before or after you upload it) in order to check to see if it includes the changes you want. Thanks for working on this!

Changed 9 years ago by robharron

In padics, fix bug with Zmod element conversion, add conversion from residue_field

comment:7 follow-up: Changed 9 years ago by robharron

  • Status changed from needs_work to needs_review

Ok, I used the hg_sage functions in sage and the patch now seems to be what it should be. Patch file updated.

comment:8 Changed 9 years ago by roed

  • Status changed from needs_review to positive_review

I don't know why the testbot showed failing tests: all tests pass for me locally.

Looks good. I'm hoping to redo conversion from the residue in a different way later, but in the mean time this approach works.

Changed 9 years ago by jdemeyer

Fix duplicate header

comment:9 Changed 9 years ago by jdemeyer

  • Authors set to Robert Harron
  • Description modified (diff)
  • Reviewers set to David Roe

comment:10 in reply to: ↑ 7 Changed 9 years ago by jdemeyer

Replying to robharron:

Ok, I used the hg_sage functions in sage and the patch now seems to be what it should be. Patch file updated.

For some reason, your patch had a double header (I don't know what you did wrong). I removed the first and re-uploaded the patch.

comment:11 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.