You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mahout.apache.org by "Pallavi Palleti (JIRA)" <ji...@apache.org> on 2008/07/08 11:53:38 UTC
[jira] Created: (MAHOUT-67) plus method in AbstractVector doesn't
work for SparseVectors
plus method in AbstractVector doesn't work for SparseVectors
------------------------------------------------------------
Key: MAHOUT-67
URL: https://issues.apache.org/jira/browse/MAHOUT-67
Project: Mahout
Issue Type: Bug
Components: Matrix
Reporter: Pallavi Palleti
Priority: Minor
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method and divide method in
AbstractVector doesn't work for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Summary: plus method and divide method in AbstractVector doesn't work for SparseVectors (was: plus method in AbstractVector doesn't work for SparseVectors)
also added divide method
> plus method and divide method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (MAHOUT-67) plus method in
AbstractVector doesn't work for SparseVectors
Posted by "Karl Wettin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611772#action_12611772 ]
karl.wettin edited comment on MAHOUT-67 at 7/8/08 1:02 PM:
-----------------------------------------------------------
You say you need to modify the cardinality. I'm not sure about what has been said about the constraints of Vector modifiers, but the code seems rather static about Vectors being compatible from the start.
Is this also about making use of the sparseness? Then how about something like this in AbstractVector:
{code:java}
===================================================================
--- core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (revision 674908)
+++ core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (working copy)
@@ -230,15 +230,16 @@
/*
* (non-Javadoc)
*
* @see org.apache.mahout.matrix.Vector#plus(org.apache.mahout.matrix.Vector)
*/
public Vector plus(Vector x) throws CardinalityException {
if (cardinality() != x.cardinality())
throw new CardinalityException();
Vector result = copy();
- for (int i = 0; i < result.cardinality(); i++)
- result.setQuick(i, getQuick(i) + x.getQuick(i));
+ for (Vector.Element e : x) {
+ result.setQuick(e.index(), getQuick(e.index()) + e.get());
+ }
return result;
}
{code}
was (Author: karl.wettin):
You say you need to modify the cardinality. I'm not sure about what has been said about the constraints of Vector modifiers, but the code seems rather static about Vectors being compatible from the start.
Is this also about making use of the sparseness? Then how about something like this in AbstractVector:
{code:java}
===================================================================
--- core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (revision 674908)
+++ core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (working copy)
@@ -230,15 +230,16 @@
/*
* (non-Javadoc)
*
* @see org.apache.mahout.matrix.Vector#plus(org.apache.mahout.matrix.Vector)
*/
public Vector plus(Vector x) throws CardinalityException {
if (cardinality() != x.cardinality())
throw new CardinalityException();
Vector result = copy();
- for (int i = 0; i < result.cardinality(); i++)
- result.setQuick(i, getQuick(i) + x.getQuick(i));
+ for (Vector.Element e : x) {
+ result.setQuick(e.index(), e.get());
+ }
return result;
}
{code}
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (MAHOUT-67) plus method in
AbstractVector doesn't work for SparseVectors
Posted by "Karl Wettin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611772#action_12611772 ]
karl.wettin edited comment on MAHOUT-67 at 7/8/08 1:04 PM:
-----------------------------------------------------------
You say you need to modify the cardinality. I'm not sure about what has been said about the constraints of Vector modifiers, but the code seems rather static about Vectors being compatible from the start.
[deleted some stuff here that made no sense]
was (Author: karl.wettin):
You say you need to modify the cardinality. I'm not sure about what has been said about the constraints of Vector modifiers, but the code seems rather static about Vectors being compatible from the start.
Is this also about making use of the sparseness? Then how about something like this in AbstractVector:
{code:java}
===================================================================
--- core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (revision 674908)
+++ core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (working copy)
@@ -230,15 +230,16 @@
/*
* (non-Javadoc)
*
* @see org.apache.mahout.matrix.Vector#plus(org.apache.mahout.matrix.Vector)
*/
public Vector plus(Vector x) throws CardinalityException {
if (cardinality() != x.cardinality())
throw new CardinalityException();
Vector result = copy();
- for (int i = 0; i < result.cardinality(); i++)
- result.setQuick(i, getQuick(i) + x.getQuick(i));
+ for (Vector.Element e : x) {
+ result.setQuick(e.index(), getQuick(e.index()) + e.get());
+ }
return result;
}
{code}
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-67) plus method in AbstractVector doesn't
work for SparseVectors
Posted by "Isabel Drost (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611731#action_12611731 ]
Isabel Drost commented on MAHOUT-67:
------------------------------------
Instead of using the following explicitly, you might consider using Java5 style notation, as vector implements Iterable:
> java.util.Iterator<Vector.Element> iterator = x.iterator();
> while (iterator.hasNext()) {
> Vector.Element element = iterator.next();
I haven't tried it, but it should be possible to use the following expression:
for (Vector.Element element : x) {
...
}
I always find this style easier to read than the explicit use of iterator()
Isabel
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-67) plus method in AbstractVector doesn't
work for SparseVectors
Posted by "Karl Wettin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611772#action_12611772 ]
Karl Wettin commented on MAHOUT-67:
-----------------------------------
You say you need to modify the cardinality. I'm not sure about what has been said about the constraints of Vector modifiers, but the code seems rather static about Vectors being compatible from the start.
Is this also about making use of the sparseness? Then how about something like this in AbstractVector:
{code:java}
===================================================================
--- core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (revision 674908)
+++ core/src/main/java/org/apache/mahout/matrix/AbstractVector.java (working copy)
@@ -230,15 +230,16 @@
/*
* (non-Javadoc)
*
* @see org.apache.mahout.matrix.Vector#plus(org.apache.mahout.matrix.Vector)
*/
public Vector plus(Vector x) throws CardinalityException {
if (cardinality() != x.cardinality())
throw new CardinalityException();
Vector result = copy();
- for (int i = 0; i < result.cardinality(); i++)
- result.setQuick(i, getQuick(i) + x.getQuick(i));
+ for (Vector.Element e : x) {
+ result.setQuick(e.index(), e.get());
+ }
return result;
}
{code}
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method and divide method in
AbstractVector doesn't work for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Attachment: MAHOUT-67.patch
I have added unit tests to show that the current implementation breaks for sparse vector plus, dot and divide operation. and my version works.
> plus method and divide method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method and divide method in
AbstractVector doesn't work for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Attachment: MAHOUT-67.patch
changed to Java5 notation style.
Also added divide method for sparse vector.
> plus method and divide method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (MAHOUT-67) plus method and divide method in
AbstractVector can be optimized for SparseVectors
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sean Owen resolved MAHOUT-67.
-----------------------------
Resolution: Fixed
Fix Version/s: 0.2
Assignee: Sean Owen
I checked, and at this point AbstractVector already does what this patch did, through use of iterateNonZero(). Marking this basically obsoleted.
> plus method and divide method in AbstractVector can be optimized for SparseVectors
> ----------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.2
>
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-67) plus method in AbstractVector doesn't
work for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611906#action_12611906 ]
Pallavi Palleti commented on MAHOUT-67:
---------------------------------------
Isabel: Sure, I will make those changes.
Karl: I couldn't get. Can you please elaborate it?
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-67) plus method and divide method in
AbstractVector doesn't work for SparseVectors
Posted by "Isabel Drost (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612228#action_12612228 ]
Isabel Drost commented on MAHOUT-67:
------------------------------------
Could you please also add a unit test that shows, that the current implementation is broken but your version works?
> plus method and divide method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method and divide method in
AbstractVector can be optimized for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Summary: plus method and divide method in AbstractVector can be optimized for SparseVectors (was: plus method and divide method in AbstractVector doesn't work for SparseVectors)
I misunderstood the sparse vector representation. I agree that the cardinality exception should be thrown if two sparse vectors' cardinality is not same. But, my implementation still holds and optimizes the computation of divide and plus operation over sparse vectors.
> plus method and divide method in AbstractVector can be optimized for SparseVectors
> ----------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method and divide method in
AbstractVector can be optimized for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Attachment: MAHOUT-67.patch
I have restored the cardinality check in dot method and also added cardinality check for plus method and so updated unit tests accordingly. As this code is optimization of existing code, the previous unit tests holds.
> plus method and divide method in AbstractVector can be optimized for SparseVectors
> ----------------------------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch, MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-67) plus method in AbstractVector doesn't
work for SparseVectors
Posted by "Pallavi Palleti (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pallavi Palleti updated MAHOUT-67:
----------------------------------
Attachment: MAHOUT-67.patch
I have overridden plus() method in SparseVector. Inorder to do this, I need to add a method called addCardinality().
Also, I found that in dot() method, it shouldn't throw cardinalityException. So, I removed that condition. Please review the code.
Thanks
Pallavi
> plus method in AbstractVector doesn't work for SparseVectors
> ------------------------------------------------------------
>
> Key: MAHOUT-67
> URL: https://issues.apache.org/jira/browse/MAHOUT-67
> Project: Mahout
> Issue Type: Bug
> Components: Matrix
> Reporter: Pallavi Palleti
> Priority: Minor
> Attachments: MAHOUT-67.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.