Re: [Bigdata-developers] MIN and MAX aggregates fixed: trac 736 (with question?)

[Jeremy J C. <jj...@sy...>]

thanks
Jeremy J Carroll
Principal Architect
Syapse, Inc.



On Oct 10, 2013, at 2:01 AM, Bryan Thompson <br...@sy...> wrote:

> Jeremy,
> 
> com.bigdata.bop.rdf.aggregate.TestMAX.test_max_with_errors (from com.bigdata.rdf.TestAll)
> 
> And also TestMIN.test_min_with_errors are failing in CI.
> 
> Bryan
> 
> On Oct 9, 2013, at 8:30 PM, "Jeremy J Carroll" <jj...@sy...> wrote:
> 
>> I have verified that MIN and MAX both inherit from INeedsMaterialization
>> and added tests that use some of the literals from TestIVComparator that need materialization.
>> 
>> Unless Jenkins says otherwise I believe the change is good
>> 
>> Jeremy J Carroll
>> Principal Architect
>> Syapse, Inc.
>> 
>> 
>> 
>> On Oct 9, 2013, at 5:04 PM, Jeremy J Carroll <jj...@sy...> wrote:
>> 
>>> This is really helpful, I was just about to go home, but think I will spend a few more minutes on this!
>>> 
>>> 
>>> Jeremy J Carroll
>>> Principal Architect
>>> Syapse, Inc.
>>> 
>>> 
>>> 
>>> On Oct 9, 2013, at 5:00 PM, Bryan Thompson <br...@sy...> wrote:
>>> 
>>>> From the ticket you mentioned -- the INeedsMaterialization interface is
>>>> used to declare which bops require materialization.  This gets lifted from
>>>> subexpressions to decide if we need to do materialization before entering
>>>> the expression. 
>>>> 
>>>>>>> SNIP<<<
>>>> There are some known issues with comparisons of inline and non-inline
>>>> IVs which MikeP will be addressing shortly.  Basically, we need an
>>>> INeedsMaterialization option for operators which always require the
>>>> materialization of non-inline IVs.  ORDER_BY and aggregation both have
>>>> this characteristic since they must run over all solutions at once and
>>>> can not re-try solutions which have failed with a
>>>> NotMaterializedException?
>>>> <https://sourceforge.net/apps/trac/bigdata/wiki/NotMaterializedException>.
>>>> As an initial step in that direction, I have defined an IVComparator
>>>> and a test suite for that class and also added a
>>>> compareLiteral(IV,Literal) to IVUtility.  While this covers some cases,
>>>> it does not cover all as demonstrated by TestIVComparator.
>>>> 
>>>>>>> SNIP<<<
>>>> 
>>>> I think that the question for MIN and MAX is just whether they are
>>>> imposing the total ordering for SPARQL over RDF Values.  Again, I would
>>>> have to look at the code. That might be an old FIXME.  Or it might be a
>>>> valid problem.  However, I think that there are DAWG test cases that cover
>>>> this.  I would check there first and make sure that there is a problem (or
>>>> an absence of test coverage).
>>>> 
>>>> 
>>>> Bryan
>>>> 
>>>> 
>>>> On 10/9/13 7:26 PM, "Jeremy J Carroll" <jj...@sy...> wrote:
>>>> 
>>>>> 
>>>>> Please glance at this code (from MIN.java or MAX.java)
>>>>> 
>>>>> ...
>>>>> 
>>>>> +           private static IVComparator comparator = new IVComparator();
>>>>> 
>>>>> Š
>>>>> 
>>>>> 
>>>>> -                /**
>>>>> -                 * FIXME This needs to use the ordering define by
>>>>> ORDER_BY. The
>>>>> -                 * CompareBOp imposes the ordering defined for the "<"
>>>>> operator
>>>>> -                 * which is less robust and will throw a type exception
>>>>> if you
>>>>> -                 * attempt to compare unlike Values.
>>>>> -                 *
>>>>> -                 * @see
>>>>> https://sourceforge.net/apps/trac/bigdata/ticket/300#comment:5
>>>>> -                 */
>>>>> -                if (CompareBOp.compare(iv, min, CompareOp.LT)) {
>>>>> +                if (comparator.compare(iv, min)<0) {
>>>>> 
>>>>>                  min = iv;
>>>>> 
>>>>>              }
>>>>> 
>>>>> 
>>>>> 
>>>>> It seems to work, but I read something about requiring materialization
>>>>> which I did not understand and chose to ignore - was that a mistake?
>>>>> 
>>>>> Jeremy J Carroll
>>>>> Principal Architect
>>>>> Syapse, Inc.
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --------------------------------------------------------------------------
>>>>> ----
>>>>> October Webinars: Code for Performance
>>>>> Free Intel webinars can help you accelerate application performance.
>>>>> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most
>>>>> from 
>>>>> the latest Intel processors and coprocessors. See abstracts and register >
>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktr
>>>>> k
>>>>> _______________________________________________
>>>>> Bigdata-developers mailing list
>>>>> Big...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/bigdata-developers
>>>> 
>>> 
>>