Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New metrics to return Inventory table with quantity (kg/GWe) in terms of cumulative power from time 0 to time of the simulation #163

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

louishartono
Copy link
Contributor

@louishartono louishartono commented Mar 6, 2020

Merging prerequisite: PR #164

Made some changes to metrics.py. Added a new metrics called "InventoryQuantityPerCumulativePower" to return the explicit inventory table with quantity in units of kg/GWe, calculated by dividing the original quantity by the cumulative sum of the electricity generated in TimeSeriesPower metric from time 0 and created the test to check the expected output as well. Feel free to give feedback or suggestions :).

@louishartono louishartono changed the title Integral power [WIP] New metrics to return Inventory table with quantity (kg/GWe) in terms of total power from time 0 to time of the simulation Mar 6, 2020
total_power = 0
for t in range(len(df1)):
realpower = total_power + df1.Value[t]
total_power += realpower
Copy link
Member

@bam241 bam241 Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
total_power += realpower
total_power += df1.Value[t]

@louishartono louishartono changed the title [WIP] New metrics to return Inventory table with quantity (kg/GWe) in terms of total power from time 0 to time of the simulation [WIP] New metrics to return Inventory table with quantity (kg/GWe) in terms of cumulative power from time 0 to time of the simulation Mar 11, 2020
@louishartono louishartono changed the title [WIP] New metrics to return Inventory table with quantity (kg/GWe) in terms of cumulative power from time 0 to time of the simulation New metrics to return Inventory table with quantity (kg/GWe) in terms of cumulative power from time 0 to time of the simulation Apr 23, 2020
Copy link
Member

@bam241 bam241 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainly formatting issues, otherwise this mostly look good.

in the test_metrics.py I would also add a single blank line to split each part of the test methods:

  • declaration of the expected value
  • declaration of the input
  • getting the observed value and testing it against the expected one

@@ -504,4 +504,58 @@ def inventory_quantity_per_gwe(expinv,power):
inv.Quantity = inv.Quantity/inv.Value
inv=inv.drop(['Value'],axis=1)
return inv


# Cumulative power from time 0 to time t in TimeSeriesPower metric
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8: 2 lines between methods


@metric(name='CumulativeTimeSeriesPower', depends=_cumpdeps, schema=_cumpschema)
def cumulative_timeseriespower(power):
"""Cumulative Timeseriespower metric returns the TimeSeriesPower metric with a cumulative sum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line < 80 chars

Comment on lines +523 to +528
power = pd.DataFrame(data={'SimId': power.SimId,
'AgentId': power.AgentId,
'Time': power.Time,
'Value': power.Value},
columns=['SimId','AgentID','Time', 'Value'])
power_index = ['SimId','Time']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no tabs :)
add a space after ,

df1['Value'] = df1['Value'].cumsum()
return df1

# Quantity per GigaWattElectric Per Cumulative Power in Inventory [kg/GWe]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 blank line between methods

return df1

# Quantity per GigaWattElectric Per Cumulative Power in Inventory [kg/GWe]
_invdeps = ['ExplicitInventory','CumulativeTimeSeriesPower']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ad space after ,

('SimId', 'O'), ('AgentId', '<i8'), ('Time', '<i8'),
('InventoryName', 'O'), ('NucId', '<i8'), ('Quantity', '<f8')]))
)
#ctsp is the CumulativeTimeSeriesPower metrics
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ctsp is the CumulativeTimeSeriesPower metrics
# ctsp is the CumulativeTimeSeriesPower metrics

], dtype=ensure_dt_bytes([
('SimId', 'O'), ('Time', '<i8'), ('Value', '<f8')]))
)
#inv is the ExplicitInventory metrics
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#inv is the ExplicitInventory metrics
# inv is the ExplicitInventory metrics

('InventoryName', 'O'), ('NucId', '<i8'), ('Quantity', '<f8')]))
)
obs = metrics.inventory_quantity_per_cumulative_power.func(inv, ctsp)
assert_frame_equal(exp, obs)

if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8 2 blank lines between methods

Comment on lines +442 to +445
(UUID('f22f2281-2464-420a-8325-37320fd418f8'), 1, 2, 'core', 922350000, 300),
(UUID('f22f2281-2464-420a-8325-37320fd418f8'), 1, 2, 'usedfuel', 922350000, 600),
(UUID('f22f2281-2464-420a-8325-37320fd418f8'), 2, 3, 'core', 922350000, 1050),
(UUID('f22f2281-2464-420a-8325-37320fd418f8'), 2, 3, 'usedfuel', 922350000, 350)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those lines are too long :)

(UUID('f22f2281-2464-420a-8325-37320fd418f8'), 2, 3, 'usedfuel', 922350000, 350)
], dtype=ensure_dt_bytes([
('SimId', 'O'), ('AgentId', '<i8'), ('Time', '<i8'),
('InventoryName', 'O'), ('NucId', '<i8'), ('Quantity', '<f8')]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, maybe put all the pair on a line ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants