[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226809#comment-13226809 ] Claudio Squarcella commented on SANDBOX-404: Hi Simone, thanks for the notice. Working on it now! Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226818#comment-13226818 ] Claudio Squarcella commented on SANDBOX-404: Hi, I found it. All the above errors were caused by the tests using null weight operations, while the compiler expects an explicit type. So as a workaround I just added casts to existing types: see the example below. Before: {code} findShortestPath( graph ).from( a ).to( b ).applyingDijkstra( null ); {code} After: {code} findShortestPath( graph ).from( a ).to( b ).applyingDijkstra( (DoubleWeightBaseOperations) null ); {code} Now, secondary issue: while looking for a solution I also noticed that {{SpanningTree}} did not have a generic type for the type of weight operations (i.e. the variable in {{MutableSpanningTree}} had explicit type {{MonoidW}}). So as a first attempt to fix the error I added {{WO}} as a generic type in {{SpanningTree}} and modified all the classes using it. Is it ok if I commit both changes together? (and since we're here, I can also do the same for {{InMemoryWeightedPath}} that still uses {{MonoidW}}). Claudio Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226859#comment-13226859 ] Simone Tripodi commented on SANDBOX-404: {quote} Now, secondary issue: while looking for a solution I also noticed that {{SpanningTree}} did not have a generic type for the type of weight operations (i.e. the variable in {{MutableSpanningTree}} had explicit type {{MonoidW}}). So as a first attempt to fix the error I added {{WO}} as a generic type in {{SpanningTree}} and modified all the classes using it. {quote} It is not an issue, it is not broken in my branch (at least). Why should it matter which type of Monoid the spanning trees are using, at APIs level? They don't add any value to execute algorithms. If you can take a look at the experimental branch, I simplified a lot also the chain builders signatures, getting rid of useless generic types. For what I've experienced, the only types that really matter are Vertices {{V}}, Edges {{E}} and Weights {{W}} (and {{Graph}} and its specializations) Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226892#comment-13226892 ] Claudio Squarcella commented on SANDBOX-404: Hi Simone, you were convincing :) I just reverted that change and only committed the solution for the issue you discovered ([see r1299232|http://svn.apache.org/viewvc?view=revisionrevision=1299232]). Claudio Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226902#comment-13226902 ] Simone Tripodi commented on SANDBOX-404: great, thanks a lot!!! instead of enjoying vacations, can you please apply the same change on the branch? It would simplify the future merge, if accepted. TIA, all the best! -Simo PS I was obviously joking ;) Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13226589#comment-13226589 ] Simone Tripodi commented on SANDBOX-404: Hi Claudio, unfortunately the patch you committed doesn't work when launching tests from CLI (OTOH works well inside eclipse), follows below the errors I got: {code} $ svn info Path: . URL: https://svn.apache.org/repos/asf/commons/sandbox/graph/trunk Repository Root: https://svn.apache.org/repos/asf Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68 Revision: 1299097 Node Kind: directory Schedule: normal Last Changed Author: cs Last Changed Rev: 1298136 Last Changed Date: 2012-03-07 22:34:21 +0100 (Wed, 07 Mar 2012) ... mvn clean test ... [ERROR] COMPILATION ERROR : [INFO] - [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/shortestpath/BellmannFordTestCase.java:[77,63] type parameters of WOorg.apache.commons.graph.shortestpath.AllVertexPairsShortestPathorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double,WO cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/spanning/KruskalTestCase.java:[71,77] type parameters of WOorg.apache.commons.graph.SpanningTreeorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/shortestpath/AStarTestCase.java:[98,65] type parameters of WOorg.apache.commons.graph.shortestpath.HeuristicBuilderorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double,org.apache.commons.graph.model.UndirectedMutableWeightedGraphorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double,WO cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/spanning/ReverseDeleteTestCase.java:[53,67] type parameters of WOorg.apache.commons.graph.SpanningTreeorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/spanning/PrimTestCase.java:[71,77] type parameters of WOorg.apache.commons.graph.SpanningTreeorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/shortestpath/DijkstraTestCase.java:[69,68] type parameters of WOorg.apache.commons.graph.WeightedPathorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [ERROR] /private/tmp/graph/src/test/java/org/apache/commons/graph/spanning/BoruvkaTestCase.java:[71,77] type parameters of WOorg.apache.commons.graph.SpanningTreeorg.apache.commons.graph.model.BaseLabeledVertex,org.apache.commons.graph.model.BaseLabeledWeightedEdgejava.lang.Double,java.lang.Double cannot be determined; no unique maximal instance exists for type variable WO with upper bounds java.lang.Object,org.apache.commons.graph.weight.Monoidjava.lang.Double,java.util.Comparatorjava.lang.Double [INFO] 7 errors [INFO] - [INFO] [INFO] BUILD FAILURE [INFO]
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13224737#comment-13224737 ] Claudio Squarcella commented on SANDBOX-404: Hi, just a quick notice to confirm that I just committed the modifications contained in [the second patch posted above|https://issues.apache.org/jira/secure/attachment/12516966/SANDBOX-404_gettingRidOfOrderedMonoid.patch] (see [r1298136|http://svn.apache.org/viewvc?view=revisionrevision=1298136]) :) Claudio Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13223081#comment-13223081 ] Simone Tripodi commented on SANDBOX-404: Hi Claudio! I still don't understand how the rename simplifies the actual scenario design: I suppose that users with the need to calculate a _Strongly Connected Component_ on their graph data are enough educated that should understand basic Algebra, if even a B.Sc. like me does I suppose everybody does :P The only way to evaluate the _if and when_ condition is IMHO listing at least one graph algorithm that requires a different Monoid to perform its execution, otherwise no needs to model something unused. Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13223150#comment-13223150 ] Claudio Squarcella commented on SANDBOX-404: Hi, one question left from my side and then I shutdown ( :) ): why would you still call {{Monoid}} something which in all the algorithms is actually used and seen as an {{Addition}}? Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13223155#comment-13223155 ] Simone Tripodi commented on SANDBOX-404: please don't shutdown because this is a matter of making the component good software, not just writing code! So I deduce that having an interface to define the behavior is not enough, users are still able to create an {{Addition}} instance and implement the multiplication operator inside - it doesn't matter how we call it... uh... Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_FromMonoidToAddition.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221877#comment-13221877 ] Simone Tripodi commented on SANDBOX-404: hola, having a look at the patch I am not really happy on how signatures heavily changed, once dropped the {{OrderedMonoid}}, but I can safety live with it. The rename instead doesn't convince me at all: {{Monoid}} perfectly reflects the monoid definition - it is an interface indeed - {{Addition}} is one monoid *instance*. What is we have to add more Monoid operations? Do you intend to create new monoid for each operation? Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221901#comment-13221901 ] Claudio Squarcella commented on SANDBOX-404: Hi! First things first: My idea is to completely get rid of {{Monoid}}, in favor of a group of interfaces directly representing operations. In the specific case, {{Addition}} would immediately take its place not semantically but *functionally*, to cover algorithm needs -- they indeed need to apply addition and not a generic monoid, so that would also increase consistency. It would look more or less like this: {code} public interface AdditionE { E sum(E e1, E e2); E zero(); E negate(E e); } {code} In case later we want to add {{Multiplication}} it will be totally independent, as explained in my first comment above. Something like: {code} public interface MultiplicationE { E multiply(E e1, E e2); E one(); // or identity(), we'll see E reciprocal(E e); } {code} As for the signature change, I did it because I would prefer not to stack interfaces on top of each other like we did with {{Zero}}, {{Semigroup}}, {{Monoid}} and {{OrderedMonoid}}. As long as we can easily write in the signatures all the individual properties we need (in the example {{Addition}} and {{Comparator}}) we can avoid to add interfaces like {{ComparableAddition}}, {{ComparableMultiplication}}, {{ComparableAdditionMultiplication}}... see my point? Concluding: I can work on {{Addition}} if, and as soon as, we agree. Ciao, Claudio Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221906#comment-13221906 ] Simone Tripodi commented on SANDBOX-404: Until there will be _real_ need of adding new Monoid implementations, I would suggest to postpone the problem and keep *your* version of the patch. Let's fight with the right weapons when we need, I wouldn't use a katana to kill a fly (unless I am Bruce Willis and I'm in Pulp Fiction :P) best and thanks! -Simo Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch, SANDBOX-404_gettingRidOfOrderedMonoid.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-404) Simplify weight model
[ https://issues.apache.org/jira/browse/SANDBOX-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221645#comment-13221645 ] Claudio Squarcella commented on SANDBOX-404: Hi Simo, good that you moved discussion here so we can focus. I don't know if I got your question correctly, but I have answers for both interpretations :) * _what if our class has to implement two different kinds of Monoid (e.g. Addition and Multiplication)?_ I propose to convert our current {{Monoid}} to {{Addition}} to represent the sum operation (which indeed is how we use it right know). Then we can add {{Multiplication}} as a completely separate interface, without the need to be backed by the same abstract monoid (although, of course, theory tells us that they are both monoids): * _what if our class has to be not just a Monoid, but also something else?_ Then we simply add a new interface. I am already thinking of clearly separating {{Monoid}}/{{Addition}} from {{Comparator}} Simplify weight model - Key: SANDBOX-404 URL: https://issues.apache.org/jira/browse/SANDBOX-404 Project: Commons Sandbox Issue Type: Improvement Components: Graph Reporter: Simone Tripodi Attachments: SANDBOX-404.patch As discussed on {{dev@}}, {{Zero}}, {{Semigroup}} and {{Monoid}} can be merged directly in one single interface -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira