Changes to AnalyzePG #40
Conversation
There was a problem hiding this comment.
This looks good. I have a lot of ideas for updating the rest of the file but they're not related to the changes you're making here. If you can merge this into develop soon then I can start adding in the code from spot check.
By the way, I clicked "approve" because even though I had some comments the changes are very small. That means once you make the changes as long as they work you can merge without showing them to me. If I had clicked "request changes" it would mean you should show me your changes to get another round of feedback.
Anyway, as they say in the software world, ship it! 👍
| :return: (*tuple*) -- date frame of PG and associated capacity for all | ||
| storage units located in zone. | ||
| """ | ||
| storage_id = [] |
There was a problem hiding this comment.
Small nitpick: could you rename this to storage_ids? I think it's easier to understand.
There was a problem hiding this comment.
I can but the function returns the power generated by all storage devices in zone and the corresponding total capacity.
| for c, bus in enumerate(self.grid.storage['gen'].bus_id.values): | ||
| if self.grid.bus.loc[bus].zone_id in self._get_zone_id(zone): | ||
| storage_id.append(c) |
There was a problem hiding this comment.
This method works just fine, but I think you can use a where clause here.
storage_ids = self.grid.bus.where(id in self.grid.storage.gen.bus_id & id in self._get_zone_id(zone))
I kind of fudged the table locations but I think you get the general idea.
If you're feeling rushed don't worry about figuring this out, but keep it as an idea for next time.
There was a problem hiding this comment.
Agree. I will implement this tomorrow
| ax.xaxis.set_major_locator(mdates.MonthLocator()) | ||
| ax.xaxis.set_major_formatter(mdates.DateFormatter('%b')) |
There was a problem hiding this comment.
Not sure. It is from @dmuldrew. I believe it set the tick mark and tick label to month-year (e.g. Jan 2016, Feb 2016, ...)
postreise/plot/analyze_pg.py
Outdated
| for t in type2label.keys(): | ||
| if t == 'solar': | ||
| pg_solar = self._get_pg(zone, ['solar'])[0].sum(axis=1) | ||
| net_demand['net_demand'] = net_demand['net_demand'] - \ | ||
| pg_solar | ||
| curtailment_solar = self._get_profile(zone, 'solar').sum( | ||
| axis=1).tolist() - pg_solar | ||
| pg_stack['sc'] = curtailment_solar | ||
| elif t == 'wind': | ||
| pg_wind = self._get_pg(zone, ['wind'])[0].sum(axis=1) | ||
| net_demand['net_demand'] = net_demand['net_demand'] - \ | ||
| pg_wind | ||
| curtailment_wind = self._get_profile(zone, 'wind').sum( | ||
| axis=1).tolist() - pg_wind | ||
| pg_stack['wc'] = curtailment_wind |
There was a problem hiding this comment.
Let's optimize some duplicate code here. Do you think this would work?
# Add solar and wind curtailment
for (t, key) in [('solar', 'sc'), ('wind', 'wc')]:
if t in type2label.keys():
pg_t = self._get_pg(zone, [t])[0].sum(axis=1)
net_demand['net_demand'] = net_demand['net_demand'] - pg_t
curtailment_t = self._get_profile(zone, t).sum(axis=1).tolist() - pg_t
pg_stack[key] = curtailment_tThere was a problem hiding this comment.
Yes it should. I will implement it tomorrow.
Do not hesitate to make changes and commit. I would say it is preferable that you do it it now instead of merging now, then creating a new branch wit a new PR and CR.
Changes can be seen running the following code: