Opened 10 years ago

Closed 10 years ago

#10477 closed enhancement (fixed)

Add a random vector constructor

Reported by: rbeezer Owned by: jason, was
Priority: minor Milestone: sage-4.7
Component: linear algebra Keywords:
Cc: kcrisman Merged in: sage-4.7.alpha3
Authors: Rob Beezer Reviewers: Felix Lawrence
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by rbeezer)

random_matrix() will build a variety of random matrices, employing the random_element() method for the base ring. This patch provides the basic functionality for creating random vectors, or random free module elements, mimicking the constructor for matrices.

This depends trivially on #10364 since both write into the same line of all.py.

Attachments (2)

trac_10477-random-vector-constructor.patch (5.8 KB) - added by rbeezer 10 years ago.
trac_10477-random-vector-constructor-v2.patch (6.4 KB) - added by rbeezer 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by rbeezer

comment:1 follow-up: Changed 10 years ago by rbeezer

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 in reply to: ↑ 1 ; follow-up: Changed 10 years ago by rbeezer

  • Status changed from needs_review to needs_work

Replying to rbeezer:

This might work better if it just called the random_element() method for vector spaces. I'm going to move this to "needs work" until I get a chance to investigate.

comment:3 in reply to: ↑ 2 Changed 10 years ago by rbeezer

  • Status changed from needs_work to needs_review

Replying to rbeezer:

This might work better if it just called the random_element() method for vector spaces. I'm going to move this to "needs work" until I get a chance to investigate.

.random_element() allows setting the density of the result (fraction of zeros), while this routine allows for sparse representations. The present routine could be extended to include a probability argument to allow for controlling the density. Run times look similar.

sage: time for i in range(5000): random_vector(QQ, 100)
CPU times: user 4.22 s, sys: 0.05 s, total: 4.27 s
Wall time: 4.28 s
sage: V=QQ^100
sage: time for i in range(5000): V.random_element()
CPU times: user 3.95 s, sys: 0.00 s, total: 3.95 s
Wall time: 3.96 s

So I think this is ready (again) for review.

comment:4 Changed 10 years ago by kcrisman

  • Cc kcrisman added

comment:5 Changed 10 years ago by flawrence

Hi Rob,

This looks good! Two comments, the second of which is petty:

  • I think the default ring should be RR rather than ZZ. I can think of lots of uses for a vector of random reals, but not as many for a vector of integers. I'm open to persuasion though!
  • Before doing degree = ring it would be good to check that degree==None. Otherwise the following slips through without raising an error:
    sage: random_vector(11,2)
    (0, 1, -3, 2, -9, -1, -1, 0, 1, 1, 1)
    

comment:6 Changed 10 years ago by rbeezer

Hi Felix,

Thanks for the catch on the degree argument. If no ring is specified, and a value is in the second slot (so assigned to the degree keyword) I have shoved it into the front-end of the argument list. A new doctest shows this in action. (Inserting a new random doctest then meant changing all of the subsequent random output.)

So with this change in the new patch we have:

Before:

sage: random_vector(10, 50)
(1, 2, 1, 1, 3, 1, -2, -1, -63, 1)

After:

sage: random_vector(10, 50)
(48, 24, 48, 33, 6, 9, 41, 14, 43, 44)

I think every one of the matrix constructors (random or otherwise) defaults to the integers. I'd guess two reasons for this - Sage's roots in number theory, or matrices (vectors) over other rings will usually play nicely with integers (in other words, the integers will coerce smoothly into lots of rings).

In any event, I used the integers only for some measure of consistency with the matrix constructors.

Rob

comment:7 Changed 10 years ago by flawrence

  • Reviewers set to Felix Lawrence
  • Status changed from needs_review to positive_review

Fair enough, it's best to be consistent with the other constructors. With the new patch, the code works well and passed long doctests. Positive review.

comment:8 Changed 10 years ago by jdemeyer

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