hama-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Edward J. Yoon" <edwardy...@apache.org>
Subject Re: Bug in Aggregator
Date Thu, 23 Jan 2014 14:25:30 GMT
Yes, it was. For example, AggregatorTest. See what happens if you set the num of tasks.

I've committed my changes. Let's improve step-by-step.

Sent from my iPhone

> On 2014. 1. 23., at 오후 10:53, Anastasis Andronidis <andronat_asf@hotmail.com>
wrote:
> 
> Sorry, can't see something easy and elegant. For me, the best approach would be to stay
on simple aggregate(name, value).
> 
> BTW, my patch has other parts that simplify the code. Are you sure that it's all written
wrong? Should I open smaller tickets to do updates? Or just drop everything? (e.g. named aggregators,
etc..)
> 
> The AbsDiffAggregator is a complicate case. I don't see any convenient way to support
it with a simple interface like vertex.aggregate(name, value); I don't see a way to interact
it with 
> 
>> On 23 Ιαν 2014, at 12:14 μ.μ., Edward J. Yoon <edwardyoon@apache.org>
wrote:
>> 
>> Yes, basically I agree with you. User should handle it. BTW, at the
>> same time, I don't want to remove AbsDiffAggregator.
>> 
>> Do you have any good idea?
>> 
>> On Thu, Jan 23, 2014 at 7:38 PM, Anastasis Andronidis
>> <andronat_asf@hotmail.com> wrote:
>>> So you do agree that the user should handle such cases or Hama should implement
different methods to keep track of previous states?
>>> 
>>>> On 23 Ιαν 2014, at 11:31 π.μ., Edward J. Yoon <edwardyoon@apache.org>
wrote:
>>>> 
>>>> Yes, AbsDiffAggregator. I've uploaded a patch.
>>>> 
>>>> On Thu, Jan 23, 2014 at 7:21 PM, Anastasis Andronidis
>>>> <andronat_asf@hotmail.com> wrote:
>>>>> Hello,
>>>>> 
>>>>> sorry I am a little bit confused.
>>>>> if I understand correctly, the problem is that we don't keep the value
of the previous superstep? Isn't this something that the user should implement on his own?
>>>>> 
>>>>> I'll take as an example the AbsDiffAggregator:
>>>>> 
>>>>> This aggregator is taking as an input, the (old value of the vertex)
- (the current value) and then it sums everything across all vertices. Isn't this the same
as making the diff inside the vertex and then sending the output to a sum aggregator?
>>>>> 
>>>>> (by the user in Vertex class)
>>>>> 
>>>>> --- in the beginning of the code ---
>>>>> startV = this.getValue();
>>>>> 
>>>>> /* Code run */
>>>>> 
>>>>> this.aggregate("sum_aggr", startV - this.getValue());
>>>>> 
>>>>> If I am correct, I think that we should document this kind of behavior,
so the users can use it. I said that I will handle the documentation on the wiki for the aggregators,
I have it on schedule for this weekend.
>>>>> 
>>>>> Cheers,
>>>>> Anastasis
>>>>> 
>>>>>> On 23 Ιαν 2014, at 2:30 π.μ., Edward J. Yoon <edwardyoon@apache.org>
wrote:
>>>>>> 
>>>>>> I found a bug in aggregator.
>>>>>> 
>>>>>> In parseMessages, you calls masterAggregation() method. Do you think
>>>>>> everything is OK?
>>>>>> 
>>>>>> /**
>>>>>> * Method to let the custom master aggregator read messages from peers
and
>>>>>> * aggregate a value.
>>>>>> */
>>>>>> @SuppressWarnings("unchecked")
>>>>>> public void masterAggregation(Text name, Writable value) {
>>>>>> String nameIdx = name.toString().split(";", 2)[1];
>>>>>> this.Aggregators.get(nameIdx).aggregate(null, value);
>>>>>> 
>>>>>> // When it's time to send the values, we can see which aggregators
are used.
>>>>>> this.aggregatorsUsed.add(nameIdx);
>>>>>> }
>>>>>> 
>>>>>> The aggregated value will be always the last value.
>>>>>> 
>>>>>> Like our old code,
>>>>>> 
>>>>>> getAggregationRunner().aggregateVertex(lastValue, vertex);
>>>>>> 
>>>>>> You should aggregates all values.
>>>>>> 
>>>>>> --
>>>>>> Best Regards, Edward J. Yoon
>>>>>> @eddieyoon
>>>> 
>>>> 
>>>> 
>>>> --
>>>> Best Regards, Edward J. Yoon
>>>> @eddieyoon
>> 
>> 
>> 
>> -- 
>> Best Regards, Edward J. Yoon
>> @eddieyoon
> 

Mime
View raw message