search

ames-market / psst

PSST - Power System Simulation Toolbox
Other
39 stars 27 forks source link

Minor bug fixes that enable reading matpower cases #7

Closed djinnome closed 4 years ago

djinnome commented 5 years ago

Hi @kdheepak

This pull request includes some very minor bug fixes that are probably due to recent changes in pandas and networkx that cropped up when trying to execute 'read_matpower()'.

For https://github.com/power-system-simulation-toolbox/psst/commit/7112324002d2ba8c0ddecdc4bcf7916cf447b767

I changed the within=NonNegativeReals to within=Reals for line 51 of generators.py as you requested.

Also, on lines 87 and 90 of model/__init__.py you had

timeseries_pmax = generator_df["PMAX"].to_dict(orient="list")

However, to_dict() no longer allows the orient keyword for Series (although it does for DataFrames)

For https://github.com/power-system-simulation-toolbox/psst/commit/a1c90d1b634dcc4090553e532bbde327e46e4164 I could not read case2383wp because it contained -Inf and Inf in the Qmin and Qmax, which raised an OverflowError instead of a ValueError so I added a check for that.

With https://github.com/power-system-simulation-toolbox/psst/commit/a1c90d1b634dcc4090553e532bbde327e46e4164 For some reason networkx no longer allows directly referencing edge attributes by key, so in order to access an edge attribute you have to go through the attr_dict first.

Lastly, for https://github.com/power-system-simulation-toolbox/psst/commit/315fc0f03d801428f4818d91ab326f173da54bcc

model/__init__.py

the idiom 

load_df = load_df or case.load

only works if load_df is None. If load_df is a dataframe, then this statement generates a ValueError because

ValueError: The truth value of a DataFrame is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all().

So to fix this, I explicitly check to see if the dataframe is None and then only perform the assignment if it is.

kdheepak commented 5 years ago

This is great! Thanks for making the PR! I think we need to add some tests that run on Travis. I can do that as a separate PR. Let me test this on my local machines and then I'll merge it.