Opened 10 years ago

Closed 9 years ago

#11796 closed defect (fixed)

inconsistency between 0.nbits() and 0.ndigits(base=2)

Reported by: marion Owned by: AlexGhitza
Priority: minor Milestone: sage-4.8
Component: basic arithmetic Keywords: ecc2011
Cc: Merged in: sage-4.8.alpha3
Authors: Paul Zimmermann Reviewers: André Apitzsch
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

There is an inconsistency between functions nbits(), bits() and digits(base=2) when applied to zero:

sage: 0.nbits()
1
sage: 0.ndigits(base=2)
0
sage: 0.bits()
[]
sage: 0.digits(base=2)
[]

Attachments (1)

trac11796.patch (976 bytes) - added by zimmerma 10 years ago.

Download all attachments as: .zip

Change History (12)

Changed 10 years ago by zimmerma

comment:1 Changed 10 years ago by zimmerma

  • Status changed from new to needs_review

I checked all doctests with Sage 4.7.1 and they all still pass. Strangely the value of nbits for 0 was never tested nor apparently used.

Paul

comment:2 Changed 10 years ago by marion

  • Status changed from needs_review to needs_work

I checked the long doctests on my machine with version 4.7.1 of sage and the following tests failed:

	sage -t -long devel/sage-test/sage/databases/database.py # 15 doctests failed
	sage -t -long devel/sage-test/sage/graphs/graph.py # 6 doctests failed
	sage -t -long devel/sage-test/sage/graphs/graph_list.py # 4 doctests failed
	sage -t -long devel/sage-test/sage/graphs/graph_database.py # 28 doctests failed
	sage -t -long devel/sage-test/sage/graphs/generic_graph.py # 4 doctests failed

Marion

comment:3 Changed 10 years ago by zimmerma

  • Status changed from needs_work to needs_info

Marion,

this is strange, since on my laptop (Core 2 Duo) with Sage 4.7.1 those 5 long doctests pass.

Please can you try again those 5 doctests? Just run for example sage -t -long databases/database.py in the right directory. Did they pass without the patch?

Paul

comment:4 Changed 10 years ago by marion

Ok, on my Sage without the patch all these 5 doctests failed too (I did not check the tests on the version I downloaded first...) I will investigate further where the problem comes from.

Marion

comment:5 Changed 10 years ago by zimmerma

  • Keywords ecc2011 added

Marion, you can also try on a computer where all doctests pass before the patch, and open a separate ticket for the problem on your computer.

Paul

comment:6 follow-up: Changed 9 years ago by aapitzsch

The problem was already mentioned in #10596. But there ndigits() was changed to return 1 instead of 0.

So, whose solution is correct and why?

comment:7 in reply to: ↑ 6 Changed 9 years ago by zimmerma

Replying to aapitzsch:

The problem was already mentioned in #10596. But there ndigits() was changed to return 1 instead of 0.

So, whose solution is correct and why?

I cannot apply the patches from #10596 to Sage 4.7:

sage: hg_sage.import_patch("/tmp/trac_10596.patch")
cd "/usr/local/sage/devel/sage" && hg status
cd "/usr/local/sage/devel/sage" && hg status
cd "/usr/local/sage/devel/sage" && hg import   "/tmp/trac_10596.patch"
applying /tmp/trac_10596.patch
patching file sage/rings/integer.pyx
Hunk #4 succeeded at 566 with fuzz 2 (offset -34 lines).
Hunk #69 FAILED at 5248
1 out of 80 hunks FAILED -- saving rejects to file sage/rings/integer.pyx.rej

What is inconsistent is that we should have 0.nbits() == len(0.bits()) == 0.ndigits(base=2) == len(0.digits(base=2)), which currently does not hold.

Now if 0.nbits() is 1, then 0.bits() should be [0]. I would prefer to return 0 and [] respectively, but first solve the inconsistency.

Paul

comment:8 Changed 9 years ago by aapitzsch

The patches from #10596 are based on Sage 4.8.alpha2.

comment:9 Changed 9 years ago by aapitzsch

  • Authors set to Paul Zimmermann
  • Reviewers set to André Apitzsch
  • Status changed from needs_info to needs_review

sage/databases/database.py was removed by #11642. All other tests pass.

comment:10 Changed 9 years ago by aapitzsch

  • Status changed from needs_review to positive_review

comment:11 Changed 9 years ago by jdemeyer

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