On 14 Jul 2011, at 12:35, Reto Bachmann-Gmür wrote:
> I agree with the principle of getting things to work first and then
> care about performance but here I think it would be a fundamentally
> flaw in the architecture that will necessarily cause very bad
> performance.
I was seeing it not as a flaw in the architecture, but rather an optimisation
problem.
Let me develop that line of thinking.
First:
would the following not solve all the problems in the test suite?
Replace
graph.addAll(sub.getGraph)
with
if (graph != sub.getGraph)
graph.addAll(sub.getGraph)
Then none of the code written in the test suite would lead to copying of
graphs, since they are always built from the the same EzMGraph.
Second, (perhaps the following is the missing justification)
Relating a node in one graph to a bnode in another does not make sense
unless you also copy some context over. Ok, here I copy all the graph: perhaps
copying the smallest context would be enough (though that will change depending
on what type of inference is running). It's an interesting issue as to what is
easiest to understand. Copying all over may be easiest to understand, and then
providing a method to produce the minimum graph would be an option, so that the
developer can decide how much he really needs.
But the main point is below:
> One can get the test in the issue (which really if a
> request for enhancement) to pass without introducing these parts.
I don't think that the tests would pass if you don't add those changes too.
The reason the code in your
tests works with constructs such as
reto.a(FOAF.Person)
-- FOAF.knows --> (
"http://bblfish.net/#hjs".uri.a(FOAF.Person)
-- FOAF.name --> "Henry Story"
-- FOAF.currentProject --> "http://webid.info/".uri
)
is because the following happens (starting from the center)
1. "http://bblfish.net/#hjs".uri.a(FOAF.Person)
a) gets transformed into
new URIRef("http://bblfish.net/#hjs";)
because the .uri method is defined on EzLiteral and there is the
implicit def string2lit(str: String) = new EzLiteral(str)
b) then it gets transformed into a RichGraphNode - let's call it
anon_rgn_hjs - because the method a(..) is defined there and because of the
implicit (call it IMP)
implicit def toRichGraphNode(resource: Resource) = {
new RichGraphNode(new GraphNode(resource, baseTc))
}
2. Then the RichGraphNode anon_rgn_hjs is filled out with the FOAF.name and
FOAF.currentProject relations
3. reto - a rich graph node - is then related by foaf knows to
anon_rgn_hjs.resource
by the following function
def -->(sub: GraphNode): RichGraphNode = {
//RichGraphNode.this + sub
-->(sub.getNode)
}
QUESTION: what happens to the graph of anon_rgn_hjs??? How does it get
added to the graph of reto?
--------
Well this occurs because of the magic you were using, which this whole issue
is questioning the wisdom of,
namely the implicit we mentioned above and named IMP:
implicit def toRichGraphNode(resource: Resource) = {
new RichGraphNode(new GraphNode(resource, baseTc))
}
Notice that the implicit created a new GraphNode, whose graph is called
"baseTc". But where does baseTc come from?
Ahah. IMP is part of a trait:
protected trait TcDependentConversions extends TcIndependentConversions {
def baseTc: TripleCollection
implicit def toRichGraphNode(resource: Resource) = {
new RichGraphNode(new GraphNode(resource, baseTc))
}
}
And EzMGraph is defined as extending that trait.
class EzMGraph(val baseTc: MGraph) extends AbstractMGraph with
TcDependentConversions
putting it together:
-------------------
When you create a new EzMGraph - call it ez - you create an object with the
toRichGraphNode() implicit, whose baseTc set to that Graph's tc collection. The
you
import ez._
to import that implicit so that it becomes available for usage. Of course that
implicit is tightly tied to ez.
So this explains then how anon_rgn_hjs gets added to ez. It's because when in
1b above when
new URIRef("http://bblfish.net/#hjs";) was transformed into a RichGraphNode
using the IMP implicit, the backing
graph of ez was added as its backing store.
Ok so all that is really clever! But just a bit too clever by half, sadly.
Remember that the point of this patch is to fix the real problem with the
current framework, which is that
currently if you have two graphs you need to keep writing the import statements
every time you wish to
change graph. Like this:
import ez1._
ez1.b_("reto") -- FOAF.homepage --> "http://bblfish.net/".uri
import ez2._
ez2.b_("hjs") -- FOAF.homepage --> "http://bblfish.net/".uri
import ez1._
ez1.b_("reto") -- FOAF.currentProject --> "http://clerezza.org/".uri
This is very error prone it seems to me.
And the reason you have that error, is because you have created an implicit
that is tied
to a hidden variable. Now I really am willing to bet some money that if you put
this to the
Scala mailing list they will award you with the discovery of a Scala
anti-pattern :-)
Henry
>
> Reto
>
> On Thu, Jul 14, 2011 at 12:23 PM, Henry Story <[email protected]> wrote:
>>
>> On 14 Jul 2011, at 11:32, Reto Bachmann-Gmür wrote:
>>
>>> The issue I can agree with addressing is 603. While I think the naming
>>> is inappropriately dramatic as it would be better described as
>>> supporting a slightly different coding style, as I commented on the
>>> issue I could agree with a resolution.
>>>
>>> Not sure how to apply your patch, it doesn't seem to work with the
>>> regular path -p (see below).
>>
>> I don't know. That is what they recommend to do on
>>
>> http://www.apache.org/dev/git.html
>>
>> I agree that they should also give some info on how to apply the patch!
>>
>>>
>>> I think the changes in the patch pertinent to the issue are the
>>> changes to the bnode and the b_ methods, I don't see how other changes
>>> like
>>>
>>> def -->(sub: GraphNode): RichGraphNode = {
>>> - //RichGraphNode.this + sub
>>> + graph.addAll(sub.getGraph)
>>> -->(sub.getNode)
>>> }
>>
>> If you have want code like
>>
>> gr.bnode("danny") -- FOAF.knows --> ( gr.bnode("reto").a(FOAF.Person) ... )
>>
>> to work then you need to allow the --> to take a GraphNode as object. It
>> should of course merge the
>> info.
>>
>> If you want to make it more efficient, you could open another issue to make
>> it more efficient. A simple
>> test of equality between the graphs would certainly help.
>>
>>
>>
>>>
>>> or
>>>
>>> +
>>> + /**
>>> + * Add one relation for each member of the iterable collection
>>> + */
>>> + def -->>>(elems: Iterable[GraphNode]): RichGraphNode = {
>>> + for (res <- elems) -->(res)
>>> + RichGraphNode.this
>>> + }
>>
>> This is just so that you can allow relations to lists of GraphNodes. Sadly
>> it can't be the same as -->> as that clashes
>> with the other methods due to Type Erasure.
>>
>>>
>>> would relate to the issue. (And as a side-note, I don't think a
>>> potentially very expensive operation like copying around the whole
>>> context of a GraphNode should happen as a side effect of the -->
>>> method)
>>
>> I think it is better to file a new issue to make the code more efficient.
>> There are different ways to do that I am sure, so that could itself give
>> rise to a debate. No need to mix issues. Simplicity is the mantra now. Right
>> ? :-)
>>
>> Henry
>>
>>
>>
>>>
>>> Reto
>>>
>>>
>>> Attempt to apply the patch:
>>> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ wget
>>> https://issues.apache.org/jira/secure/attachment/12486181/0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch
>>> --2011-07-14 11:14:53--
>>> https://issues.apache.org/jira/secure/attachment/12486181/0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch
>>> Resolving issues.apache.org... 140.211.11.121
>>> Connecting to issues.apache.org|140.211.11.121|:443... connected.
>>> HTTP request sent, awaiting response... 200 OK
>>> Length: 10792 (11K) [text/plain]
>>> Saving to: `0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch'
>>>
>>> 100%[===============================================================================================>]
>>> 10,792 --.-K/s in 0s
>>>
>>> 2011-07-14 11:14:54 (21.5 MB/s) -
>>> `0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch'
>>> saved [10792/10792]
>>>
>>> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ patch -p0
>>> < 0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch
>>> patching file
>>> b/parent/platform.security.foafssl/test/src/main/scala/org/apache/clerezza/foafssl/test/WebIDTester.scala
>>> Hunk #1 FAILED at 33.
>>> Hunk #2 FAILED at 122.
>>> Hunk #3 FAILED at 130.
>>> Hunk #4 FAILED at 148.
>>> Hunk #5 FAILED at 172.
>>> Hunk #6 FAILED at 199.
>>> Hunk #7 FAILED at 215.
>>> Hunk #8 FAILED at 223.
>>> Hunk #9 FAILED at 602.
>>> Hunk #10 FAILED at 709.
>>> Hunk #11 FAILED at 743.
>>> 11 out of 11 hunks FAILED -- saving rejects to file
>>> b/parent/platform.security.foafssl/test/src/main/scala/org/apache/clerezza/foafssl/test/WebIDTester.scala.rej
>>> patching file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EzMGraph.scala
>>> Hunk #1 FAILED at 54.
>>> Hunk #2 FAILED at 72.
>>> 2 out of 2 hunks FAILED -- saving rejects to file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EzMGraph.scala.rej
>>> patching file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/Preamble.scala
>>> Hunk #1 FAILED at 105.
>>> 1 out of 1 hunk FAILED -- saving rejects to file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/Preamble.scala.rej
>>> patching file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala
>>> Hunk #1 FAILED at 225.
>>> 1 out of 1 hunk FAILED -- saving rejects to file
>>> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala.rej
>>> patching file
>>> b/parent/rdf.scala.utils/src/test/scala/org/apache/clerezza/rdf/scala/utils/EzMGraphTest.scala
>>> Hunk #1 FAILED at 111.
>>> Hunk #2 FAILED at 120.
>>> Hunk #3 FAILED at 142.
>>> 3 out of 3 hunks FAILED -- saving rejects to file
>>> b/parent/rdf.scala.utils/src/test/scala/org/apache/clerezza/rdf/scala/utils/EzMGraphTest.scala.rej
>>> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ svn status
>>> ? b
>>> ? 0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch
>>>
>>> On Thu, Jul 14, 2011 at 11:05 AM, Henry Story <[email protected]>
>>> wrote:
>>>> Since there is a delay in the release I think it would be worth
>>>> considering a little
>>>> bit more closely the patches I submitted recently:
>>>>
>>>> - CLEREZZA-599 - EzMGraph imports too many implicits
>>>> - CLEREZZA-600 - remove need to define Preamble in code using
>>>> rdf.scala.utils
>>>> - CLEREZZA-603 - EzMGraph fails when working with more than one instance
>>>>
>>>> These are all very small patches I filed as requested. Each one is very
>>>> easy to understand and self
>>>> contained. CLEREZZA-603 is the one that makes the usefulness of the others
>>>> even clearer than what
>>>> they are by themselves.
>>>>
>>>> Henry Story
>>>>
>>>> Social Web Architect
>>>> http://bblfish.net/
>>>>
>>>>
>>
>> Social Web Architect
>> http://bblfish.net/
>>
>>
Social Web Architect
http://bblfish.net/
