Bugzilla – Bug 2458
GetTotalEnergyConsumption should not be const
Last modified: 2017-03-21 10:21:48 EDT
double GetTotalEnergyConsumption (void) is declared const, but it shouldn't. Its use-case is to get the energy consumed by a node at the end of the simulation, but the consumed energy measure could have been updated a LONG time ago (as in the case of vehicles motors). A more correct procedure is to update the consumed energy measurement inside the function, but this means that one must not declare it as const. The alternative is to provide a temporary measurement, but this is questionable. Personally, I'd rather have the function as non-const.
Hi, I am new to ns-3 (getting familiar with the code base and everything) and would like to start contribution by working on this bug. I might need a little help considering it is my first bug fix attempt in ns-3. So in order to update the consumed energy measurement inside the function (after I make the function non-const), I would have to do something similar to the updation in void SetCurrentA (double current) function ? Thanks, Surya.
(In reply to tssurya from comment #1) > Hi, > > I am new to ns-3 (getting familiar with the code base and everything) and > would like to start contribution by working on this bug. I might need a > little help considering it is my first bug fix attempt in ns-3. > > So in order to update the consumed energy measurement inside the function > (after I make the function non-const), I would have to do something similar > to the updation in void SetCurrentA (double current) function ? > > Thanks, > Surya. I think that Tommaso is raising a question about the base class API for some possible implementation that does lazy updating. Maybe he can clarify, before we discuss any possible patch.
Hi, it's exactly what I meant. Look at the (wrong) code in SimpleDeviceEnergyModel. GetTotalEnergyConsumption returns m_totalEnergyConsumption, which is updated if and only if SetCurrentA is called. To have a real measurement, one should either: 1) call SetCurrentA (GetCurrentA ()) (this forces an update) and then GetTotalEnergyConsumption or 2) modify GetTotalEnergyConsumption function to consider the time between the last model update and the actual time or 3) modify the object status (i.e., update m_totalEnergyConsumption) and return it. Option 1 is idiotic because it forces the user to use a trick. Option 2 can be done, but I like more option 3. Tbh I don't remember why I declared "questionable" the option 2 besides the obvious personal preferences. (In reply to Tom Henderson from comment #2) > (In reply to tssurya from comment #1) > > Hi, > > > > I am new to ns-3 (getting familiar with the code base and everything) and > > would like to start contribution by working on this bug. I might need a > > little help considering it is my first bug fix attempt in ns-3. > > > > So in order to update the consumed energy measurement inside the function > > (after I make the function non-const), I would have to do something similar > > to the update in void SetCurrentA (double current) function ? > > > > Thanks, > > Surya. > > I think that Tommaso is raising a question about the base class API for some > possible implementation that does lazy updating. Maybe he can clarify, > before we discuss any possible patch.
(In reply to Tommaso Pecorella from comment #3) > Hi, > > it's exactly what I meant. > > Look at the (wrong) code in SimpleDeviceEnergyModel. > GetTotalEnergyConsumption returns m_totalEnergyConsumption, which is updated > if and only if SetCurrentA is called. > To have a real measurement, one should either: > 1) call SetCurrentA (GetCurrentA ()) (this forces an update) and then > GetTotalEnergyConsumption or > 2) modify GetTotalEnergyConsumption function to consider the time between > the last model update and the actual time or > 3) modify the object status (i.e., update m_totalEnergyConsumption) and > return it. > > Option 1 is idiotic because it forces the user to use a trick. > Option 2 can be done, but I like more option 3. > Tbh I don't remember why I declared "questionable" the option 2 besides the > obvious personal preferences. This is an instance of a general problem of how to handle lazy state updates with const members. In this case (and in general) I believe the solution could be your option 3) by making m_lastUpdateTime and m_totalEnergyConsumption mutable, updating them inside the method, and keeping the const on the method.
(In reply to Tom Henderson from comment #4) > (In reply to Tommaso Pecorella from comment #3) > > > > 3) modify the object status (i.e., update m_totalEnergyConsumption) and > > return it. > > > > Option 1 is idiotic because it forces the user to use a trick. > > Option 2 can be done, but I like more option 3. > > Tbh I don't remember why I declared "questionable" the option 2 besides the > > obvious personal preferences. > > This is an instance of a general problem of how to handle lazy state updates > with const members. In this case (and in general) I believe the solution > could be your option 3) by making m_lastUpdateTime and > m_totalEnergyConsumption mutable, updating them inside the method, and > keeping the const on the method. OK, So I just change these two variable declarations in the class and make them mutable and using the equations in the void SetCurrentA (double current) function and update it within the method and return it. Will modify the code and work on the patch asap.
Created attachment 2810 [details] proposed patch