-
Notifications
You must be signed in to change notification settings - Fork 40
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
Create grid object from different sources #85
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
- Tests are running on my system
- Design makes sense to me
@danielolsen I have a question regarding naming of file design.py. Could that be part of the helpers.py ? I don't fully understand the name design for this module. |
@kasparm my intention was a module that is used to 'design' new scenarios to be run, via the change_tables. Right now the only public function is |
powersimdata/input/design.py
Outdated
from postreise.process import const | ||
from postreise.process.transferdata import download | ||
from powersimdata.input.profiles import _read_data | ||
from powersimdata.scenario.helpers import interconnect2name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like these are unnecessary imports that I added when creating this module? I remember I was running into circular dependency issues initially, these might have been cruft left over from trying to circumvent those.
return data_frame | ||
|
||
|
||
def add_column_to_data_frame(data_frame, column_dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this function both modifies the input dataframe and returns it. I think it might be more clear if it did one or the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I will change that. Keep you updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The opreation is done in place now. Docstring has been updated.
@@ -0,0 +1,2 @@ | |||
class MATReader(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a placeholder for reading from input.mat
, yeah?
powersimdata/input/mpc_reader.py
Outdated
from powersimdata.input.helpers import csv_to_data_frame | ||
|
||
|
||
class MPCReader(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming of the MPCReader
and MATReader
objects confused me a bit. I think it would help if the docstring for this object made it more clear that it's reading MPC attributes, but from CSVs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. We can come up with better names too. Any idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSVReader?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the MATReader? Do you want to change the name too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That one I'm more comfortable with, because it does read from a MAT file.
@danielolsen We don't need to address the changes here. |
@kasparm what about The private functions aren't used by any other modules yet, but I agree that if/once they are, they'd fit better in helpers.py. I am agnostic as to where they should live now. |
@danielolsen name sounds good. |
On my local machine, I rebased The
There are no differences in |
f0efee6
to
69762c1
Compare
My rebased/re-fixed version pushed (with approval from @rouille). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
f887e30
to
08b68e1
Compare
a274fdd
to
69762c1
Compare
8f801b2
to
47568a2
Compare
Purpose
Refactor
Grid
oject to allow the use of different models (e.g. TAMU) based on different data formats.What is the code doing
It builds the grid as we kmow it. The code now allows to build the grid from different models. usa_tamu is the only one implemented so far. Building the grid from the mat files used by MATPOWER/MOST (input.mat) is the next step.
MPC files structure have been modified. Only csv files are used now to build the grid. The information we used in the aux files have been transfered into the appropriate csv files. Also, the csv files have been correctly formatted. Note that the GenMWMin and GenMWMax columns in the
plant
data frame (attribute of theGrid
object) have been dropped. The GenMWMin and GenMWMax values from the aux files is now used inplace of the Pmin and Pmax values in the csv file for renewables generators (hydro, solar and wind) only. Since, there is no more GenMWMin and GenMWMax column some refactoring was necessary. I you look at issue #83, you will see that some refactoring in PreREISE and PostREISE is necessary too. I will create the corresponding PRs soon.Where to look
grid.py is the interface to build the grid.
usa_tamu_model.py encloses the
TAMU
object. It is a grid model.abstract_grid.py is the abstract class defining the attributes of the
Grid
object.mpc_reader.py encloses the MPCReader object. It reads MPC files format.
mat_reader encloses an empty
MATReader
object. Implementation will come in a different PR.helpers.py has a couple of functions used throughout the construction of the grid.
mock_grid.py has a
MockGrid
object. It is adapted from theMockGrid
object written by @danielolsen in PostREISE. I believe that this mock object sould live in powersimdata. Hence, I have switch the import in test_design.pytest_grid.py tests some function used in the building of the grid.
Time estimate
It does not do anything new. The code looks very different though. it will take some time to review.