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: |
Description (last modified by )
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)
Change History (10)
Changed 10 years ago by
comment:1 follow-up: ↓ 2 Changed 10 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 10 years ago by
- Status changed from needs_review to needs_work
comment:3 in reply to: ↑ 2 Changed 10 years ago by
- 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
- Cc kcrisman added
comment:5 Changed 10 years ago by
Hi Rob,
This looks good! Two comments, the second of which is petty:
- I think the default ring should be
RR
rather thanZZ
. 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 thatdegree==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)
Changed 10 years ago by
comment:6 Changed 10 years ago by
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
- 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
- Merged in set to sage-4.7.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
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.