Start and Stop functionality with Failure and Repair model

Start and Stop functionality with Failure and Repair model

[Tommaso Pecorella, 2014-03-22 17:24:10]

Hi,
first bunch of comments.

However, I have one general comment: why we don't add the StartStop bit directly
in the Object model? An object can safely start in "on" state and, if it's not
using the functionality, just forget about it.

Secondly, I'm also checking how to use the StartStop with the Energy model (I'd
suggest to have an example about that as well).

Third thing... why the StartStop is in the failure-repair model ?
I mean... I'd see it better in the core module.
Starting and stopping something isn't just about failures and repairs, it's also
about energy depletion.

Cheers,

T.

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
File src/failure-repair/doc/failure-repair.rst (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
src/failure-repair/doc/failure-repair.rst:1: Example Module Documentation
It doesn't seems like an example module...

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
src/failure-repair/doc/failure-repair.rst:16: model.
... I guess you forgot to clean the example text ...

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
src/failure-repair/doc/failure-repair.rst:22: Start and Stop Functionality
General comment. I think that the Start/Stop functionality (and the FailreRepair
model) should be documented very well.

This documentation is more like a skeleton, so I'd suggest to improve it,
perhaps also using class diagrams.

Last point: I'd explain how to integrate this with the Energy model, as it will
be the very first thing asked by the users.

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/failu...
File src/failure-repair/model/failure-repair-model.cc (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/failu...
src/failure-repair/model/failure-repair-model.cc:178: Time startedStateInterval
= m_timeInfo.TimeUnitConstructor (m_workingStateDuration->GetValue ());
This is a workaround: it works around the fact that we don't have a RNG creating
a "time" with a particular distribution...

I'd rather use this:
http://www.nsnam.org/docs/doxygen/classns3_1_1_time.html#a7a89549cebc1c5234f3...

This could make the whole TimeUnitInformation class unnecessary.

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/failu...
File src/failure-repair/utils/failure-repair-time-tracker.cc (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/failu...
src/failure-repair/utils/failure-repair-time-tracker.cc:78:
m_timeInfo.UpdateTimeUnitInformation (unit);
What m_timeInfo is used for ?
It seems a zombie variable tbh...

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
File src/failure-repair/utils/simple-start-stop-object.h (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
src/failure-repair/utils/simple-start-stop-object.h:22:
#define	SIMPLE_START_STOP_OBJECT_H
tab...

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
src/failure-repair/utils/simple-start-stop-object.h:30: class
SimpleStartStopObject : public Object, public StartStopFunctionality
This object seems to be used only in the failure model example and test. I'd
suggest to move the class there, as it's not meant to be used from other
modules.

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/start...
File src/failure-repair/utils/start-stop-functionality.h (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/start...
src/failure-repair/utils/start-stop-functionality.h:22:
#define	START_STOP_FUNCTIONALITY_H
tab :)

[Peter Barnes, 2014-03-26 23:44:56]

I've got a better way to handle printing Time in a specific unit, which I'll
push soon.

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/time-...
File src/failure-repair/model/time-unit-information.h (right):

https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/time-...
src/failure-repair/model/time-unit-information.h:34: struct TimeUnitInformation
I've got a better way to handle this, which I'll push soon.

[Peter Barnes, 2014-04-03 01:50:18]

Done, r10688, ad36a71c737c

On 2014/03/26 23:44:56, Peter Barnes wrote:
> I've got a better way to handle printing Time in a specific unit, which I'll

[Vedran Miletić, 2014-04-06 15:16:28]

On 2014/03/22 17:24:10, Tommaso Pecorella wrote:
> Hi,
> first bunch of comments.
> 
> However, I have one general comment: why we don't add the StartStop bit
directly
> in the Object model? An object can safely start in "on" state and, if it's not
> using the functionality, just forget about it.
> 

Nicola and Alina were strongly against this approach during the meeting.

> Secondly, I'm also checking how to use the StartStop with the Energy model
(I'd
> suggest to have an example about that as well).
> 

Sure. Do you have any suggestions on that?

> Third thing... why the StartStop is in the failure-repair model ?
> I mean... I'd see it better in the core module.
> Starting and stopping something isn't just about failures and repairs, it's
also
> about energy depletion.
> 

I agree. We can move the mixin to the core, and leave failure-repair stuff
outside.

> Cheers,
> 
> T.
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
> File src/failure-repair/doc/failure-repair.rst (right):
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
> src/failure-repair/doc/failure-repair.rst:1: Example Module Documentation
> It doesn't seems like an example module...
> 

Oops. Fixed that.

>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
> src/failure-repair/doc/failure-repair.rst:16: model.
> ... I guess you forgot to clean the example text ...
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/doc/failure...
> src/failure-repair/doc/failure-repair.rst:22: Start and Stop Functionality
> General comment. I think that the Start/Stop functionality (and the
FailreRepair
> model) should be documented very well.
> 
> This documentation is more like a skeleton, so I'd suggest to improve it,
> perhaps also using class diagrams.
> 

Absolutely. I'm trying to get us to agree on the design before I start
documenting it.

> Last point: I'd explain how to integrate this with the Energy model, as it
will
> be the very first thing asked by the users.
> 

Sure, go ahead.

>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/failu...
> File src/failure-repair/model/failure-repair-model.cc (right):
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/model/failu...
> src/failure-repair/model/failure-repair-model.cc:178: Time
startedStateInterval
> = m_timeInfo.TimeUnitConstructor (m_workingStateDuration->GetValue ());
> This is a workaround: it works around the fact that we don't have a RNG
creating
> a "time" with a particular distribution...
> 
> I'd rather use this:
>
http://www.nsnam.org/docs/doxygen/classns3_1_1_time.html#a7a89549cebc1c5234f3...
> 

Fixed.

> This could make the whole TimeUnitInformation class unnecessary.
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/failu...
> File src/failure-repair/utils/failure-repair-time-tracker.cc (right):
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/failu...
> src/failure-repair/utils/failure-repair-time-tracker.cc:78:
> m_timeInfo.UpdateTimeUnitInformation (unit);
> What m_timeInfo is used for ?
> It seems a zombie variable tbh...
> 

Removed completely.

>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
> File src/failure-repair/utils/simple-start-stop-object.h (right):
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
> src/failure-repair/utils/simple-start-stop-object.h:22:
> #define	SIMPLE_START_STOP_OBJECT_H
> tab...
> 

Fixed.

>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/simpl...
> src/failure-repair/utils/simple-start-stop-object.h:30: class
> SimpleStartStopObject : public Object, public StartStopFunctionality
> This object seems to be used only in the failure model example and test. I'd
> suggest to move the class there, as it's not meant to be used from other
> modules.
> 

Sure.

>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/start...
> File src/failure-repair/utils/start-stop-functionality.h (right):
> 
>
https://codereview.appspot.com/66990047/diff/1/src/failure-repair/utils/start...
> src/failure-repair/utils/start-stop-functionality.h:22:
> #define	START_STOP_FUNCTIONALITY_H
> tab :)

Fixed.

Uploading as patch set 3. Let's try to agree on the design first, everything
else can follow easily.

Regards,
Vedran

[Vedran Miletić, 2014-04-06 15:38:13]

On 2014/04/03 01:50:18, Peter Barnes wrote:
> Done, r10688, ad36a71c737c
> 
> On 2014/03/26 23:44:56, Peter Barnes wrote:
> > I've got a better way to handle printing Time in a specific unit, which I'll

Hi Peter,

I'm not using this at the moment since TimeUnitInformation got replaced by
Time::FromDouble. However, I see a possibility for pretty printing total uptime
and downtime in a specific time unit which could be quite neat.

Regards,
Vedran