arfc / d3ploy

A collection of Cyclus manager archetypes for demand driven deployment
BSD 3-Clause "New" or "Revised" License
4 stars 11 forks source link

Two tables for power in .sqlite #196

Closed robfairh closed 5 years ago

robfairh commented 5 years ago

There are two tables for Power. The first one is 'timeseriespower', which starts in the beginning of the simulation. image The first one is 'timeseriessupplypower', it starts when the first reactor produces power. image

I don't know if all the functions are consistent in the choice of table. For calculating the 'under_supply' it uses the second one which leads to an inaccurate value:

case6_arch_power

The first values should be consider as under supply as there is no power generated during those times. borrar4

This issue can be closed when we make sure that the use of both tables is consistent and we have a value for supply (even if it is 0) when the reactor is deployed and not when it starts producing power.

robfairh commented 5 years ago

@gwenchee any ideas about this?

robfairh commented 5 years ago

@FlanFlanagan

gwenchee commented 5 years ago

Timeseriespower is a table that’s always been there and timeseriessupplypower was something that we added.

Are you saying that the timeseriessupplypower is used in timeseriesinst? The calc_diff function?

robfairh commented 5 years ago

Timeseriesinst uses calc_demand and calc_supply for those calculations. So the deployment of facilities shouldn't be affected by the presence of that timeseriessupplypower. But I am not completely sure. That's why I am asking. The issue that I noticed is that tester.py makes use of timeseriessupplypower which doesn't start at t=0. I am also asking if we should do something about it or not.

gwenchee commented 5 years ago

If there is no power supply in time step 0 then it should be fine to use it?

On Mon, 4 Mar 2019 at 12:15 PM, robfairh notifications@github.com wrote:

Timeseriesinst uses calc_demand and calc_supply for those calculations. So the deployment of facilities shouldn't be affected by the presence of that timeseriessupplypower. But I am not completely sure. That's why I am asking. The issue that I noticed is that tester.py makes use of timeseriessupplypower which doesn't start at t=0. I am also asking if we should do something about it or not.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/arfc/d3ploy/issues/196#issuecomment-469383755, or mute the thread https://github.com/notifications/unsubscribe-auth/AcCTfHLQ4dWTA7drEJUCHDjACxQz4U2fks5vTXC-gaJpZM4bXmyk .

-- Gwendolyn J. Chee Master's Student Department of Nuclear, Plasma and Radiological Engineering University of Illinois at Urbana-Champaign Email: gchee2@illinois.edu

robfairh commented 5 years ago

timeseriessupplypower starts at the first timestep where power is not 0. Even though, reactors were deployed, there is no power produced by them.

If we are actually looking at the supply of power, then the table should start at timestep 0 or when the first reactor is deployed. If we are looking at the under supply of power, then we are not taking into consideration the timesteps where the reactors are deployed and don't produce any power.

I think that both tables should be the same. I don't know if I am explaining the problem properly.

gwenchee commented 5 years ago

What should we do about this issue @robfairh

gwenchee commented 5 years ago

we should edit tester so that it reflects the power supply is 0 in time steps 1 and 2 since the reactor facilities are deployed at time step 1

robfairh commented 5 years ago

This is cycamore/src/reactor.cc now:

image

I think it should be like this:

image

What do you think @gwenchee ?? I am not brave enough ...

robfairh commented 5 years ago

I did it and it seems to be working. cycamore_unit_tests worked fine.

Timeseriespower:

image

Timeseriessupplypower:

image

Should I make a PR to cycamore? @gwenchee

gwenchee commented 5 years ago

Ya do it!

robfairh commented 5 years ago

addressed by https://github.com/cyclus/cycamore/pull/499

FlanFlanagan commented 5 years ago

closed with merging of that PR