Clojure

Subvecs of primitive vectors cannot be reduced

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor 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)

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Timothy Baldridge added a comment -

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 - 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.
Hide
Permalink
Andy Fingerhut added a comment -

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 - 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
Mike Anderson added a comment -

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 - 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
Mike Anderson added a comment -

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 - 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 -

Patch for CLJ-1082, containing 3 commits

Show
Mike Anderson added a comment - Patch for CLJ-1082, containing 3 commits
Hide
Permalink
Andy Fingerhut added a comment -

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.

  1. Check out a fresh repo
    $ git clone git://github.com/clojure/clojure.git
    $ cd clojure
  1. 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
  1. Create a branch for yourself
    $ git checkout -b fix-clj-1082
  1. 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 - 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.
  1. Check out a fresh repo $ git clone git://github.com/clojure/clojure.git $ cd clojure
  1. 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
  1. Create a branch for yourself $ git checkout -b fix-clj-1082
  1. 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.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.