[#CLJ-1082] Subvecs of primitive vectors cannot be reduced - Clojure JIRA

Details

Type: Defect
Status: Open
Priority: Minor
Resolution: Unresolved
Affects Version/s: None
Fix Version/s: None
Component/s: None
Labels:
None
Environment:
1.7.0-08, OS X 10.8

Patch:
Code and Test
Approval:
Vetted

Description

I could reproduce on current 10/05 git.

Reduce doesn't work on subvecs of primitive vectors.
Root cause seems to be that RT/subvec and APersistentVector$SubVector make assumptions about implementation

If reduce on a subvec doesn't work then neither will nice ops like fold.

How to cause:

(let [prim-vec (into (vector-of :long) (range 10000))]
(reduce + (subvec prim-vec 1 500)))

->> ClassCastException clojure.core.Vec cannot be cast to clojure.lang.PersistentVector clojure.lang.APersistentVector$SubVector.iterator (APersistentVector.java:523)

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

clj-1082.patch

20/Jan/13 6:42 PM

4 kB

Mike Anderson

Activity

Descending order - Click to sort in ascending order

Hide

Permalink

Andy Fingerhut added a comment - 21/Jan/13 1:11 AM

Mike, your patch clj-1082.patch applies cleanly to latest master for me, so looks like you found one way to do it.

Another would be as follows, and closer to the directions on the JIRA workflow page: http://dev.clojure.org/display/design/JIRA+workflow (but not identical). Note that these commands would work on Mac OS X or Linux. I'm not sure what the correct corresponding command would be on Windows for the "git am" step below, unless that just happens to work because Windows and/or git implement the input redirection with "<" somehow.

Check out a fresh repo
$ git clone git://github.com/clojure/clojure.git
$ cd clojure

Apply the patch for CLJ-1076 to the master branch. This step isn't on the JIRA workflow page.
$ git am --keep-cr -s < clj-1076-fix-tests-on-windows-patch-v1.txt

Create a branch for yourself
$ git checkout -b fix-clj-1082

After editing to make your changes, commit them to the current fix-clj-1082 branch
$ git commit -a -m "fixed annoying bug, refs #42"

From there on down it is the same as the instructions on the JIRA workflow page. The "git format-patch master --stdout > file.patch" will create a patch for the changes you have made in the current branch fix-clj-1082 starting from the master branch, which has the CLJ-1076 fix because of the 'git am' command above.

Show

Andy Fingerhut added a comment - 21/Jan/13 1:11 AM Mike, your patch clj-1082.patch applies cleanly to latest master for me, so looks like you found one way to do it. Another would be as follows, and closer to the directions on the JIRA workflow page: http://dev.clojure.org/display/design/JIRA+workflow (but not identical). Note that these commands would work on Mac OS X or Linux. I'm not sure what the correct corresponding command would be on Windows for the "git am" step below, unless that just happens to work because Windows and/or git implement the input redirection with "<" somehow.

Check out a fresh repo $ git clone git://github.com/clojure/clojure.git $ cd clojure

Apply the patch for CLJ-1076 to the master branch. This step isn't on the JIRA workflow page. $ git am --keep-cr -s < clj-1076-fix-tests-on-windows-patch-v1.txt

Create a branch for yourself $ git checkout -b fix-clj-1082

After editing to make your changes, commit them to the current fix-clj-1082 branch $ git commit -a -m "fixed annoying bug, refs #42"

Hide

Permalink

Mike Anderson added a comment - 20/Jan/13 6:42 PM

Patch for CLJ-1082, containing 3 commits

Show

Mike Anderson added a comment - 20/Jan/13 6:42 PM Patch for CLJ-1082, containing 3 commits

Hide

Permalink

Mike Anderson added a comment - 20/Jan/13 6:41 PM

Attached a patch that I created with:

git format-patch winfix --stdout HEAD~3..HEAD > clj-1082.patch

Does this do the trick? I had to use the HEAD~3..HEAD to just get the most recent 3 commits and exclude the pprint changes that I needed in order to build on Windows.

Show

Mike Anderson added a comment - 20/Jan/13 6:41 PM Attached a patch that I created with: git format-patch winfix --stdout HEAD~3..HEAD > clj-1082.patch Does this do the trick? I had to use the HEAD~3..HEAD to just get the most recent 3 commits and exclude the pprint changes that I needed in order to build on Windows.

Hide

Permalink

Mike Anderson added a comment - 20/Jan/13 5:14 AM

I have a fix for this and a regression test in the tree below:

https://github.com/mikera/clojure/tree/winfix

Not sure how best to make this into a patch though - I also have the pprint fix in here (CLJ-1076)

Show

Mike Anderson added a comment - 20/Jan/13 5:14 AM I have a fix for this and a regression test in the tree below: https://github.com/mikera/clojure/tree/winfix Not sure how best to make this into a patch though - I also have the pprint fix in here (CLJ-1076)

Hide

Permalink

Andy Fingerhut added a comment - 27/Nov/12 12:09 PM

There is no code or ticket for this yet, but I wanted to mention that I've begun working on an implementation of RRB-Tree (Relaxed Radix Balanced Tree) vectors for Clojure (see discussion at https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/xnbtzTVEK9A). Assuming it is no big deal to get reducers to work on such vectors, subvec could be implemented in O(log n) time in such a way that the result was the same concrete type of vector as you started with, and thus reducers would work on them, too.

It would mean O(log n) time for subvec instead of today's O(1), but this would likely fix other limitations that exist today with subvec's, e.g. CLJ-761.

Show

Andy Fingerhut added a comment - 27/Nov/12 12:09 PM There is no code or ticket for this yet, but I wanted to mention that I've begun working on an implementation of RRB-Tree (Relaxed Radix Balanced Tree) vectors for Clojure (see discussion at https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/xnbtzTVEK9A). Assuming it is no big deal to get reducers to work on such vectors, subvec could be implemented in O(log n) time in such a way that the result was the same concrete type of vector as you started with, and thus reducers would work on them, too. It would mean O(log n) time for subvec instead of today's O(1), but this would likely fix other limitations that exist today with subvec's, e.g. CLJ-761.

Hide

Permalink

Timothy Baldridge added a comment - 27/Nov/12 11:52 AM

Confirmed to be broken on master. Vetting. Not sure what it's going to take to fix this, however. If this is of intrest for you, you might want to help push it along by providing a patch.

Show

Timothy Baldridge added a comment - 27/Nov/12 11:52 AM Confirmed to be broken on master. Vetting. Not sure what it's going to take to fix this, however. If this is of intrest for you, you might want to help push it along by providing a patch.

People

Assignee:

Unassigned

Reporter:

Ghadi Shayban

Vote (0)

Watch (2)

Dates

Created:

05/Oct/12 6:49 PM

Updated:

26/Jan/13 7:29 AM