Support local+native usage #477

jenhagg · 2021-05-06T23:19:19Z

Purpose

Add the remaining code changes so that the framework can be used locally, outside docker, on any os, using a local REISE.jl installation.

What the code is doing

Factor out launch_scenario into subclasses for each use case, and similarly for extracting output.
Add State.refresh method so state changes have up to date information without re-initializing the instance
Split up configuration into classes, making it easy to override values specific to a given environment
various cleanup

Testing

Ran a simulation end to end on windows. Ran into issues due to using julia 1.6 but upgrading PyCall and CSV packages fixed the issues.
Ran simulation on MacOS using julia 1.5 (there are issues with 1.6)
Ran simulation in plug to verify no regressions

Usage Example/Visuals

The PR for documentation is Breakthrough-Energy/docs#62. For testing locally, I changed the name of the data directory to ScenarioDataLocal (in both PowerSimData and REISE.jl) so it won't collide with the existing data from the server.

Time estimate

40 mins

powersimdata/data_access/context.py

powersimdata/utility/config.py

powersimdata/data_access/launcher.py

ahurli

I like the new classes for the different ways to access the engine! I'm curious as your thoughts about how this design can support multiple engines in the future. It looks like we're only supporting REISE.jl in our local implementation (as opposed to including REISE like the ssh implementation does), which is probably fine for now, but as we add new engines (e.g. switch, PowerSimulations.jl), would we add new launchers (e.g. NativeSwitchLauncher, 'NativeREISEjlLauncher`) or do you think there's a better way of doing that?

ahurli · 2021-05-07T18:21:31Z

powersimdata/utility/config.py

+
+def get_deployment_mode():
+    mode = os.getenv("DEPLOYMENT_MODE")
+    if mode is None:


I know making the server deployment type the default is easiest for the team now, but should we think about moving to having local be the default at some point? As our user base grows, that seems more intuitive/easier-to-use to me.

Yeah it's unclear to me what the default should be in the future, since the docker roadmap and (corresponding user stories) is still being scoped out. But it's something to keep in mind.

jenhagg · 2021-05-07T22:41:39Z

I like the new classes for the different ways to access the engine! I'm curious as your thoughts about how this design can support multiple engines in the future. It looks like we're only supporting REISE.jl in our local implementation (as opposed to including REISE like the ssh implementation does), which is probably fine for now, but as we add new engines (e.g. switch, PowerSimulations.jl), would we add new launchers (e.g. NativeSwitchLauncher, 'NativeREISEjlLauncher`) or do you think there's a better way of doing that?

Good point - I think we'll have to refactor a bit for that. Ideally if we have a standard interface to each engine, I'm hoping we can call them the same way, just with a different configuration. Maybe something like NativeLauncher(engine='switch') or equivalent. But the signatures you mentioned might also work - if we make it so that new classes like that can be defined almost entirely by inheritance, it could be just as simple to add. Thinking here of a class hierarchy like this:

class NativeLauncherBase:
class HttpLauncherBase:
class REISEjlBase:
# implementations here

and subclassing via

class NativeREISEjlLauncher(REISEjlBase, NativeLauncherBase):
    # inheritance does all the work
    pass

Not sure exactly if/how this would work, but just sharing potential design, mainly based on this post super considered super

powersimdata/data_access/launcher.py

powersimdata/scenario/create.py

powersimdata/scenario/execute.py

powersimdata/scenario/state.py

powersimdata/utility/config.py

kasparm · 2021-05-14T17:51:02Z

Running into the following error when trying to run a scenario:

--> Launching simulation on local
Traceback (most recent call last):
  File "test.py", line 65, in <module>
    process_run = scenario.launch_simulation()
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/scenario/execute.py", line 178, in launch_simulation
    return self._launcher.launch_simulation(threads, solver, extract_data)
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/data_access/launcher.py", line 75, in launch_simulation
    return self._launch(threads, solver, extract_data)
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/data_access/launcher.py", line 196, in _launch
    return app.launch_simulation(self.scenario.scenario_id, threads, solver)
TypeError: launch_simulation() takes 1 positional argument but 3 were given

rouille · 2021-05-14T17:55:28Z

Running into the following error when trying to run a scenario:

--> Launching simulation on local
Traceback (most recent call last):
  File "test.py", line 65, in <module>
    process_run = scenario.launch_simulation()
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/scenario/execute.py", line 178, in launch_simulation
    return self._launcher.launch_simulation(threads, solver, extract_data)
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/data_access/launcher.py", line 75, in launch_simulation
    return self._launch(threads, solver, extract_data)
  File "/Users/kmueller/EGM/temp/testPSDWindows/PowerSimData/powersimdata/data_access/launcher.py", line 196, in _launch
    return app.launch_simulation(self.scenario.scenario_id, threads, solver)
TypeError: launch_simulation() takes 1 positional argument but 3 were given

What REISE.jl did you check out?

kasparm · 2021-05-14T17:57:21Z

Good point, not jon/windows.

rouille · 2021-05-14T17:58:22Z

Good point, not jon/windows.

This is one of this cross-package feature we love so much!

kasparm · 2021-05-14T18:34:27Z

Anybody go this setup to run a scenario? For me it hangs with the status "running".

jenhagg · 2021-05-14T18:51:30Z

Anybody go this setup to run a scenario? For me it hangs with the status "running".

That's farther than I got

rouille · 2021-05-14T18:56:00Z

Anybody go this setup to run a scenario? For me it hangs with the status "running".

What are you running?

kasparm · 2021-05-14T18:57:51Z

@rouille https://breakthrough-energy.github.io/docs/powersimdata/index.html#creating-a-scenario

rouille · 2021-05-14T19:08:54Z

@rouille https://breakthrough-energy.github.io/docs/powersimdata/index.html#creating-a-scenario

You could try to run a Scenatio in ERCOT (see this example)

jenhagg · 2021-05-14T20:26:22Z

Currently I'm able to run the simulation using julia once it's prepared. Narrowed down the segfault (on MacOS) to this line when we try to load julia modules within python. Not sure if I'm the only one experiencing this. Edit: it works with julia 1.5

powersimdata/data_access/launcher.py

BainanXia · 2021-05-17T17:44:36Z

powersimdata/data_access/launcher.py

+            None, which translates to gurobi
+        :param bool extract_data: whether the results of the simulation engine should
+            automatically extracted after the simulation has run. This defaults to True.
+        :return: (*subprocess.Popen*) or (*requests.Response*) - either the


or (dict) ?

BainanXia · 2021-05-17T17:45:37Z

powersimdata/data_access/launcher.py

+    :raises ValueError: if invalid solver provided
+    """
+    solvers = ("gurobi", "glpk")
+    if solver is not None and solver.lower() not in solvers:


Should we also check solver is a str similarly as we did in _check_threads?

I'm not as worried about that here, since it will still fail due to not matching one of the predefined solvers or not having a .lower() method. Although, if you're referring to the error message we would get, then I could add it.

Yeah, I was thinking about the error message.

BainanXia · 2021-05-17T17:51:54Z

powersimdata/data_access/launcher.py

+
+
+class HttpLauncher(Launcher):
+    def _launch(self, threads=None, solver=None, extract_data=True):


Do we need extract_data here, given it is not passed to request call anyway.

Yeah each _launch method has to have the same signature, since we call it the same way (from Launcher.launch_simulation) without knowing which subclass it is. I think we might end up passing it to the request eventually though.

BainanXia · 2021-05-17T17:52:29Z

powersimdata/data_access/launcher.py

+
+
+class NativeLauncher(Launcher):
+    def _launch(self, threads=None, solver=None, extract_data=True):


Same comment for extract_data.

BainanXia · 2021-05-17T18:01:52Z

powersimdata/scenario/execute.py

        :param bool extract_data: whether the results of the simulation engine should
            automatically extracted after the simulation has run. This defaults to True.
-        :param str solver: the solver used for optimization. This defaults to
-            None, which translates to gurobi
        :return: (*subprocess.Popen*) or (*requests.Response*) - either the


or (dict) ?

BainanXia · 2021-05-17T18:05:08Z

powersimdata/scenario/execute.py


-        :raises NotImplementedError: if not running in container mode
+        :return: (*dict*) -- progress information, or None


Maybe we should elaborate the returns for different launchers, SSH, HTTP, native?

BainanXia · 2021-05-17T18:35:27Z

powersimdata/utility/config.py

+    if mode == "1" or mode.lower() == "container":
+        return DeploymentMode.Container
+    if mode == "2" or mode.lower() == "local":
+        return DeploymentMode.Local


Why we need two assignment patterns for mode: numbers and string, i.e. 1 == Container 2 == Local ?

Hmm, I guess one reason is to decouple the values we set for the environment variable from the internal details. Not sure of a concrete example of when we need this, but feels like it's more flexible if the numbers don't imply any particular behavior/semantics.

rouille · 2021-05-18T16:00:00Z

What is the status of the tests?

Does it successfully run on Windows? (@jon-hagg and @danielolsen)
Does it successfully run on Mac? (@kasparm)
Does it successfully run with plug? (@jon-hagg )
Does it run with the client/server set up (@BainanXia and @danielolsen)?
By successfully, I mean creating, running and annlyzing a scenario (full integration test)

jenhagg · 2021-05-18T17:56:37Z

What is the status of the tests?

I've done successful tests natively on windows/mac and using plug.

rouille

I like it like that

…nchers

jenhagg requested review from danielolsen, ahurli, rouille and BainanXia May 6, 2021 23:19

jenhagg self-assigned this May 6, 2021

jenhagg added this to the The 4th be with you milestone May 6, 2021

jenhagg force-pushed the jon/windows branch from 70ea030 to 902c12a Compare May 6, 2021 23:23

rouille reviewed May 7, 2021

View reviewed changes

powersimdata/data_access/context.py Outdated Show resolved Hide resolved

rouille reviewed May 7, 2021

View reviewed changes

powersimdata/utility/config.py Outdated Show resolved Hide resolved

rouille reviewed May 7, 2021

View reviewed changes

powersimdata/data_access/launcher.py Show resolved Hide resolved

rouille reviewed May 7, 2021

View reviewed changes

powersimdata/data_access/launcher.py Show resolved Hide resolved

rouille reviewed May 7, 2021

View reviewed changes

powersimdata/data_access/launcher.py Outdated Show resolved Hide resolved

ahurli reviewed May 7, 2021

View reviewed changes

jenhagg marked this pull request as ready for review May 12, 2021 19:59

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/data_access/launcher.py Show resolved Hide resolved

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/scenario/create.py Show resolved Hide resolved

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/scenario/execute.py Show resolved Hide resolved

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/scenario/execute.py Show resolved Hide resolved

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/scenario/state.py Outdated Show resolved Hide resolved

rouille reviewed May 13, 2021

View reviewed changes

powersimdata/utility/config.py Show resolved Hide resolved

jenhagg mentioned this pull request May 13, 2021

refactor: make api available outside flask context Breakthrough-Energy/REISE.jl#130

Merged

jenhagg force-pushed the jon/windows branch 3 times, most recently from 63ffb49 to 126193e Compare May 14, 2021 17:31

Jon Hagg added 3 commits May 14, 2021 10:55

feat: modular config pattern

88699a9

refactor: split launch_simulation into subclasses

e3a1fac

refactor: split up other launcher methods

c5bf5e6

Jon Hagg added 4 commits May 14, 2021 11:03

chore: address PR comments

179e9ca

feat, docs: provision local directory and fix docstrings

3f04222

feat: configure root dir using config file

0860227

fix: print statements mentioning the server

c84cacb

jenhagg force-pushed the jon/windows branch from 1d13a93 to c84cacb Compare May 14, 2021 18:03

BainanXia reviewed May 17, 2021

View reviewed changes

powersimdata/data_access/launcher.py Show resolved Hide resolved

BainanXia reviewed May 17, 2021

View reviewed changes

powersimdata/data_access/launcher.py Outdated Show resolved Hide resolved

BainanXia reviewed May 17, 2021

View reviewed changes

rouille approved these changes May 18, 2021

View reviewed changes

chore: improve docstrings and consolidate return type for non ssh lau…

27c0c56

…nchers

jenhagg force-pushed the jon/windows branch from de537bc to 27c0c56 Compare May 18, 2021 22:44

jenhagg merged commit 6d26b2f into develop May 18, 2021

jenhagg deleted the jon/windows branch May 18, 2021 22:47

danielolsen mentioned this pull request Jun 11, 2021

chore: merge develop into master for v0.4.2 release #498

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support local+native usage #477

Support local+native usage #477

jenhagg commented May 6, 2021 •

edited

Loading

ahurli left a comment

ahurli May 7, 2021

jenhagg May 7, 2021

jenhagg commented May 7, 2021 •

edited

Loading

kasparm commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

jenhagg commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

rouille commented May 14, 2021

jenhagg commented May 14, 2021 •

edited

Loading

BainanXia May 17, 2021

BainanXia May 17, 2021

jenhagg May 17, 2021 •

edited

Loading

BainanXia May 17, 2021

BainanXia May 17, 2021

jenhagg May 17, 2021

BainanXia May 17, 2021

BainanXia May 17, 2021

BainanXia May 17, 2021

BainanXia May 17, 2021

jenhagg May 17, 2021

rouille commented May 18, 2021

jenhagg commented May 18, 2021

rouille left a comment



		class HttpLauncher(Launcher):
		def _launch(self, threads=None, solver=None, extract_data=True):



		class NativeLauncher(Launcher):
		def _launch(self, threads=None, solver=None, extract_data=True):


		:raises NotImplementedError: if not running in container mode
		:return: (dict) -- progress information, or None

Support local+native usage #477

Support local+native usage #477

Conversation

jenhagg commented May 6, 2021 • edited Loading

Purpose

What the code is doing

Testing

Usage Example/Visuals

Time estimate

ahurli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenhagg commented May 7, 2021 • edited Loading

kasparm commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

jenhagg commented May 14, 2021

rouille commented May 14, 2021

kasparm commented May 14, 2021

rouille commented May 14, 2021

jenhagg commented May 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenhagg May 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rouille commented May 18, 2021

jenhagg commented May 18, 2021

rouille left a comment

Choose a reason for hiding this comment

jenhagg commented May 6, 2021 •

edited

Loading

jenhagg commented May 7, 2021 •

edited

Loading

jenhagg commented May 14, 2021 •

edited

Loading

jenhagg May 17, 2021 •

edited

Loading