Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » EMF-IncQuery » Warning about cartesian product seems unnecessary in some cases
Warning about cartesian product seems unnecessary in some cases [message #1006202] Wed, 30 January 2013 07:46 Go to next message
Joost van Pinxten is currently offline Joost van Pinxten
Messages: 50
Registered: November 2012
Member
I really love the great feedback that the IncQuery editor is giving me on developing the pattern queries.

However, I just used the following pattern (Literal is the superclass of IntegerLiteral, BoolLiteral and has no other subclasses):

pattern literalType(literal : Literal, type) ={
	IntegerLiteral(literal);
	type == "integer";
} or {
	BoolLiteral(literal);
	type == "bool";
}


And this will warn me of a Cartesian product as the literal and type constraints are isolated. As I want to propagate the type through the expressions (e.g. comparing two ints yields a bool, adding/subtracting two ints yields something of type int again), I would like to annotate the tree with such kind of information. Although the Cartesian product warning is correct, it is a Cartesian product of a list combined with exactly one value (a type string literal).

Am I missing a crucial point here? Is there actually a potential memory or performance problem here?

About my actual intended goal: I'd like to make an equivalent of an expression walker with these patterns to validate (and hopefully transform) the expressions used in my DSL.

Perhaps I need to use some derived feature for this, (like in the derivedFeatures example) but I do not have the experience to tell if this is going to be a good call if I would also be using IncQuery for validations and transformations.
Re: Warning about cartesian product seems unnecessary in some cases [message #1006217 is a reply to message #1006202] Wed, 30 January 2013 08:39 Go to previous messageGo to next message
Istvan Rath is currently offline Istvan Rath
Messages: 54
Registered: July 2009
Member
Hi,

the first thing I'd recommend is to stick to the general best practice of always explicitly specifying types for your header variables. Even though you can (and are encouraged to) use "mouse hover" to discover the automatically inferred type of your variables (in this case, EString for "type"), but we've found that it helps to discover many typical problems if you explicitly indicate what type each variable should have.

Quote:
Am I missing a crucial point here? Is there actually a potential memory or performance problem here?


Yes! This pattern will instruct IncQuery to track all EString instances in your model and then compute all Literal - EString pairs, with a final post-filter operation whereby the end result is calculated as you described. For large models and performance critical applications, such patterns should generally be avoided.

I hope this helps.
Re: Warning about cartesian product seems unnecessary in some cases [message #1006243 is a reply to message #1006217] Wed, 30 January 2013 09:50 Go to previous messageGo to next message
Istvan Rath is currently offline Istvan Rath
Messages: 54
Registered: July 2009
Member
Quote:
This pattern will instruct IncQuery to track all EString instances in your model and then compute all Literal - EString pairs, with a final post-filter operation whereby the end result is calculated as you described. For large models and performance critical applications, such patterns should generally be avoided.


Let me correct myself - the performance problem would only happen if you included the equality check inside a check() expression. In your case, the filtering operation is more efficient and in this sense, the warning is probably too "strict". As I can see, Gabor has already created a ticket to exclude such cases: https://bugs.eclipse.org/bugs/show_bug.cgi?id=399497
Re: Warning about cartesian product seems unnecessary in some cases [message #1006245 is a reply to message #1006202] Wed, 30 January 2013 09:55 Go to previous messageGo to next message
Gabor Bergmann is currently offline Gabor Bergmann
Messages: 16
Registered: July 2009
Junior Member
Joost, please disregard what István just said.

Joost van Pinxten wrote on Wed, 30 January 2013 07:46

lthough the Cartesian product warning is correct, it is a Cartesian product of a list combined with exactly one value (a type string literal).

Am I missing a crucial point here? Is there actually a potential memory or performance problem here?

No, you are completely right, there is no performance issue here. We just did not think of this case, and neglected to add a specific exemption from the Cartesian rule of the validator for the case of constant literals. This is a TODO for us in the future: https://bugs.eclipse.org/bugs/show_bug.cgi?id=399497
In the meantime, you can happily ignore the warning and use your pattern.

Joost van Pinxten wrote on Wed, 30 January 2013 07:46

Perhaps I need to use some derived feature for this, (like in the derivedFeatures example) but I do not have the experience to tell if this is going to be a good call

Essentially, if you would like to access the set of inferred types from plain Java code as conveniently as myExpression.getInferredTypes(), then you need to create a (query-based) derived feature for it, using the exact same patterns. If you do not need this added convenience, you can use the IncQuery matcher API with the patterns you are defining here, and then there is no need to fiddle around with injecting derived features into the metamodel.

However, if you wish to calculate some of these types by a complicated algorithm that you express in Java only, then (non-query based) derived features are the clear choice.

Istvan Rath wrote on Wed, 30 January 2013 08:39
Hi,
the first thing I'd recommend is to stick to the general best practice of always explicitly specifying types for your header variables.

Don't do that! Generally good advice, but not in this case. EString and java.lang.String are not the same (you can think of EString as something like subtype), and "bool" is a String, but not an EString (unless it also appears as an EString attribute of some EObject), so adding EString as a type constraint may discard your matches.

Istvan Rath wrote on Wed, 30 January 2013 08:39

Yes! This pattern will instruct IncQuery to track all EString instances in your model

No, it won't. We are actually a bit more clever than that.
Re: Warning about cartesian product seems unnecessary in some cases [message #1006314 is a reply to message #1006245] Wed, 30 January 2013 14:19 Go to previous messageGo to next message
Istvan Rath is currently offline Istvan Rath
Messages: 54
Registered: July 2009
Member
Quote:
Joost, please disregard what István just said.


I reckon Gabor's (somewhat rude, but you be the judge) statement refers to my first (and truly incorrect) answer.

Quote:

Quote:
the first thing I'd recommend is to stick to the general best practice of always explicitly specifying types for your header variables.

Don't do that! Generally good advice, but not in this case. EString and java.lang.String are not the same (you can think of EString as something like subtype), and "bool" is a String, but not an EString (unless it also appears as an EString attribute of some EObject), so adding EString as a type constraint may discard your matches.


Which, in my view, means that this pattern is at least a borderline case between what can be considered good practice and something to avoid (unless absolutely necessary). Especially considering the fact that the type inferencer reports EString as the inferred EMF type of the variable in question.

In summary I think that "scalar" (i.e. non model element) variables that are not coming from a model attribute value is an area of the IncQuery Pattern Language that we need to work on a bit more. And in the mean time, users should take extra care while using them.
Re: Warning about cartesian product seems unnecessary in some cases [message #1006501 is a reply to message #1006314] Thu, 31 January 2013 09:08 Go to previous messageGo to next message
Gabor Bergmann is currently offline Gabor Bergmann
Messages: 16
Registered: July 2009
Junior Member
Istvan Rath wrote on Wed, 30 January 2013 20:19
Quote:
Joost, please disregard what István just said.


I reckon Gabor's (somewhat rude, but you be the judge) statement refers to my first (and truly incorrect) answer.

True. It seems by the time my answer was approved by moderators, you have already corrected your first statement. And sorry about the harsh choice of words, I just wanted to dissuade Joost really strongly from following your initial advice.


Istvan Rath wrote on Wed, 30 January 2013 20:19

In summary I think that "scalar" (i.e. non model element) variables that are not coming from a model attribute value is an area of the IncQuery Pattern Language that we need to work on a bit more. And in the mean time, users should take extra care while using them.

Seconded.
Created Bugzilla task: https://bugs.eclipse.org/bugs/show_bug.cgi?id=399620
Re: Warning about cartesian product seems unnecessary in some cases [message #1008242 is a reply to message #1006202] Mon, 11 February 2013 12:24 Go to previous messageGo to next message
Joost van Pinxten is currently offline Joost van Pinxten
Messages: 50
Registered: November 2012
Member
Thanks for the replies, I've been fiddling with the patterns a little bit more this afternoon.

Due to your replies I have come to understand that there are two types of 'constants' in patterns;

- Plain Java ones (like the "bool" string constant)
- EMF Enumeration values (like PrimitiveType::bool)

The first type will be instantiated wherever needed and then passed as a result; however, second type will only be matched if there is an instance of that enumeration value (and for each enumeration value) in the model the pattern is applied to.

I'd really like to work with EMF enumeration values, as this will give 'hard' errors; opposed to the soft errors that would occur when the string comparison fails (i.e. misspelling/typo/different case).
Re: Warning about cartesian product seems unnecessary in some cases [message #1008248 is a reply to message #1008242] Mon, 11 February 2013 13:08 Go to previous messageGo to next message
Abel Hegedus is currently offline Abel Hegedus
Messages: 91
Registered: July 2009
Member
You didn't include a question, yet I would like to answer an implicit one.

As far as I can see, Enumerations are different from Java constants and EDataType values. You can use Enumeration literals in your patterns and it will match even if there is no such element in the model. You can find such an example in the RecordRoleValue pattern that is used for a query-based feature in our query result snapshot models (http://git.eclipse.org/c/incquery/org.eclipse.incquery.git/tree/tests/org.eclipse.incquery.testing.queries/src/org/eclipse/viatra2/emf/incquery/testing/queries/matchRecord.eiq#n90)

Note, that this pattern also results in a Cartesian warning in the current version, but it DOES work and will return the Enumeration values as matches when invoked, even though the model does not include any attribute that has such a value.
Re: Warning about cartesian product seems unnecessary in some cases [message #1008305 is a reply to message #1008248] Mon, 11 February 2013 15:24 Go to previous messageGo to next message
Joost van Pinxten is currently offline Joost van Pinxten
Messages: 50
Registered: November 2012
Member
Thanks for the heads up Abel. However, since I've deduced the behavior that I described from experiments, I thought I'd recheck. So I've just run this through the interpreter (Query Browser that is):

Example 1, with string literals
pattern literalType(literal : Literal, type ) = {
	IntegerLiteral(literal);
	type == "integer";
} or {
	BoolLiteral(literal);
	type == "boolean";
} or {
	StringLiteral(literal);
	type == "string";
} or {
	IntegerArrayLiteral(literal);
	type == "integerArray";
}


Example 2, with EMF enum literals
pattern literalType2(literal : Literal, type : Type ) = {
	IntegerLiteral(literal);
	type == Type::integer;
} or {
	BoolLiteral(literal);
	type == Type::bool;
} or {
	StringLiteral(literal);
	type == Type::string;
} or {
	IntegerArrayLiteral(literal);
	type == Type::integerArray;
}


Example 2 does NOT return me any matches, while example 1 returns 15 (obviously on the same EMF instance model).

This should be considered a bug then? Or am I missing the point here? Smile FYI, the Type enum is also never referenced from an attribute, it is solely used to propagate types up the expression tree through the patterns used above.
Re: Warning about cartesian product seems unnecessary in some cases [message #1008307 is a reply to message #1008305] Mon, 11 February 2013 16:02 Go to previous messageGo to next message
Abel Hegedus is currently offline Abel Hegedus
Messages: 91
Registered: July 2009
Member
Try with the following header:
pattern literalType3(literal : Literal, type)


This might work because enumeration literals are actually not instances of the enumeration itself, so forcing the type can lead to an empty result set.
Re: Warning about cartesian product seems unnecessary in some cases [message #1008309 is a reply to message #1008307] Mon, 11 February 2013 16:11 Go to previous messageGo to next message
Joost van Pinxten is currently offline Joost van Pinxten
Messages: 50
Registered: November 2012
Member
Awesome, apparently I had a brainfart and missed this (actually tried it before you mentioned it). But yes, now it works perfectly!

So, enforcing the type in the header means that it must be in the model (exactly similar to Type(type) then?). Just checking the value for an instance of the Type enumeration will infer the type correctly, without implying that it should be present in the model...

Although this is a supposed to be a fringe use case (although I don't really agree with that), I would like to see this behavior documented somewhere Wink. Perhaps this discussion has enough information, but maybe a use case where some kind of grammar is checked by employing this kind of pattern would be a nice addition to the tutorials?
Re: Warning about cartesian product seems unnecessary in some cases [message #1008393 is a reply to message #1008309] Tue, 12 February 2013 05:03 Go to previous message
Gabor Bergmann is currently offline Gabor Bergmann
Messages: 16
Registered: July 2009
Junior Member
Joost, you are completely right. We will try to review the behaviour of data value variables, because the current semantics are more confusing than useful. So in future versions it is likely that explicit EDatatype checks would not pose such problems (i.e. you could add ":Type" to the header freely). Then once we adjusted the behaviour, we should turn around to documenting it more explicitly Smile
https://bugs.eclipse.org/bugs/show_bug.cgi?id=399620
Previous Topic:Finding all elements of an association in a subtree
Next Topic:EMF-IncQuery Eclipse.org migration is complete
Goto Forum:
  


Current Time: Thu Aug 28 03:12:49 EDT 2014

Powered by FUDForum. Page generated in 0.02185 seconds