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

refactor: move generation of matfile to separate function #406

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

danielolsen
Copy link
Contributor

Purpose

Decouple the matfile generation from the client/server setup. Closes #364.

What the code is doing

Code doesn't functionally change, it's just reorganized.

Testing

Not tested yet to prepare a scenario, but tested for creating a matfile from a base grid.

Usage Example/Visuals

>>> from powersimdata.input.grid import Grid
>>> from powersimdata.scenario.helpers import export_case_mat
>>> grid = Grid('Texas')
Reading bus.csv
Reading plant.csv
Reading gencost.csv
Reading branch.csv
Reading dcline.csv
Reading sub.csv
Reading bus2sub.csv
Reading zone.csv
>>> export_case_mat(grid, "dummy.mat")

Time estimate

5 minutes to understand, maybe longer to decide exactly where you think this should live.

@danielolsen danielolsen self-assigned this Feb 25, 2021
Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

Make sense.

@rouille
Copy link
Collaborator

rouille commented Feb 25, 2021

I would create a new module in PowerSimData/input next to grid.py since it can be used outside the scenario framework and it is an input of the simulation

@danielolsen
Copy link
Contributor Author

I would create a new module in PowerSimData/input next to grid.py since it can be used outside the scenario framework and it is an input of the simulation

Propose a location and I will move it. I have no attachment to the current one, I just wanted to decouple the function somehow. Wherever we put it, it would probably be good to have a convenience method on the Grid object that calls the function.

@rouille
Copy link
Collaborator

rouille commented Feb 25, 2021

I would create a new module in PowerSimData/input next to grid.py since it can be used outside the scenario framework and it is an input of the simulation

Propose a location and I will move it. I have no attachment to the current one, I just wanted to decouple the function somehow. Wherever we put it, it would probably be good to have a convenience method on the Grid object that calls the function.

powersimdata.input.case_mat. I like the name of the function (export_case_mat).

I would wait that model is merged in before creating a method in the Grid class that creates the case file.

@danielolsen
Copy link
Contributor Author

powersimdata.input.case_mat. I like the name of the function (export_case_mat).

I would wait that model is merged in before creating a method in the Grid class that creates the case file.

Once a Grid is instantiated, does it matter where it came from in order to write the tables into a matfile?

@rouille
Copy link
Collaborator

rouille commented Feb 25, 2021

powersimdata.input.case_mat. I like the name of the function (export_case_mat).
I would wait that model is merged in before creating a method in the Grid class that creates the case file.

Once a Grid is instantiated, does it matter where it came from in order to write the tables into a matfile?

No it does not. I said to wait because we did quite some modifications in the Grid class in ben/create and it would be nice to avoid conflicts. If you are eager to implement the method I will handle them.

@danielolsen
Copy link
Contributor Author

Once a Grid is instantiated, does it matter where it came from in order to write the tables into a matfile?

No it does not. I said to wait because we did quite some modifications in the Grid class in ben/create and it would be nice to avoid conflicts. If you are eager to implement the method I will handle them.

I am not in a particular rush. I needed to do this for some of my own work, and I used an ugly workaround, and in the process I saw how simple the change could be so I decided to get the ball rolling on a PR.

Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

Beautiful

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.

Improve flexibility of simulation engine input preparation process
3 participants