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

Adding event hooks for the GUI #953

Merged
merged 1 commit into from
Jun 9, 2019
Merged

Adding event hooks for the GUI #953

merged 1 commit into from
Jun 9, 2019

Conversation

tcstewar
Copy link
Collaborator

The idea here is to allow people to write functions that will automatically get called by nengo_gui at various different times. In particular, nengo_gui will look for functions with the following names:

  • def on_step(sim): called every simulation time step
  • def on_start(sim): called just before the first time step, after the user presses the Play button
  • def on_pause(sim): called when the pause button is pressed
  • def on_continue(sim): called when the Play button is pressed after a pause
  • def on_close(sim): called when the page is closed

In each case, the active Simulator object is passed in as the only argument.

There are two main use cases where I've found this handy. First, if the Nengo model is controlling a robot, it is extremely useful to be able to send a motors.stop() command when you pause the simulation. I've also used the on_start and on_close functions to initiate and shut down communication between the nengo model and a robot.

Second, the on_step(sim) is needed if I want to update a display with information I can only get from the Simulator. For example, my https://github.com/tcstewar/nengo_learning_display repository uses this to make graphs of the functions being learned by nengo learning rules, and the graphs update as the learning rule adjusts the weights.

So, I definitely think hooks like this are useful for nengo. I also like the simplicity of this implementation. However, I could see other syntax being suggested (or other ways of marking functions to indicate that they should be used as hooks like this).

@jgosmann
Copy link
Collaborator

This seems like a useful feature, but I would be careful about the exact API to not lock us in into some variant that later proofs as not as flexible, extensible, or in some other way problematic. At least on other approach comes to mind that I would like to discuss: explicitly registering hooks. This could be done with a decorator:

@nengo_gui.register_hook('on_step')
def update_learning_plots(sim):
    # code

or

@nengo_gui.on_step
def update_learning_plots(sim):
    # code

Because a decorator is essentially a function, it could also be used this way:

def update_learning_plots(sim):
    # code

def some_initialization_function(...):
    nengo_gui.register_hook('on_step')(update_learning_plots)  # or
    nengo_gui.on_step(update_learning_plots)

Advantages of this approach (that I can immediately think of):

  • Can easily be written to allow multiple functions registered for a hook.
  • Less magical, makes it more explicit what is happening, no reserved function names that might be used by the user without being aware of what is happening.
  • Is explicit about the dependence on nengo_gui (if nengo_gui cannot be imported or the code isn't running in nengo_gui, it might be easier to setup a different code path to provide the same functionality)
  • Could be extended to allow deregistering hook functions.

Disadvantages (that I can immediately think of):

  • More complex

Also, how would either of this integrate with IPythonViz?

@tbekolay
Copy link
Member

tbekolay commented Apr 18, 2018

I agree that hooks are a nice thing to have (though we should be careful about the performance implications of them). I also prefer @jgosmann's approach for the reasons he stated; additionally the bare function approach reminds me of pytest's hooks, which while convenient, are annoying to discover (I always have to google it, and the docs page with the hooks listed is not great). Putting the hooks in a nengo_gui.hooks namespace would help discoverability a lot.

@tcstewar
Copy link
Collaborator Author

tcstewar commented Apr 18, 2018

At least on other approach comes to mind that I would like to discuss: explicitly registering hooks.

Fair enough. Let me take a quick stab at that implementation too -- it's not that complicated, and it does let there be a list of multiple hooks easily.

Also, how would either of this integrate with IPythonViz?

I've never tried it, but it should work fine -- all we're doing is calling an extra function now and then. The hooks are all on a per-page basis, so multiple windows should be fine. I'm not quite sure when IPythonViz should call page.close(), but that's all I can think of that'd be a bit weird....

@tcstewar
Copy link
Collaborator Author

So, I just started trying to implement the decorator approach, and ran into a pretty big problem. Using a decorator, the hook will get registered globally, so now if I have two different pages open, it'll run for both of them. With the approach I was doing here, the registration is on a per-page basis, so we don't have this problem.

I'll see if I can do something where the decorator somehow detects that it's being run from inside a particular page.... Or another option would be something like @__page__.on_step as the decorator...

@jgosmann
Copy link
Collaborator

The ExecutionEnvironment already modifies a bunch of global state. So that could potentially be used to either keep track of the page currently executed or to keep track of hooks registered while the code is executing. Or things could be injected via code_locals into the exec function. Or we could implement the decorator as a noop and inspect the AST.

Certainly not as straight-forward as implementing magical functions, but it could be done if we really want it.

(Side note: I don't like that the model code can mess with the global state of the nengo_gui main application. It would be nice to have more separation there, but not sure how that could be achieved.)

@tbekolay
Copy link
Member

Using a decorator, the hook will get registered globally, so now if I have two different pages open, it'll run for both of them.

There's definitely a way to implement this such that that doesn't happen. Previously, you were getting the hooks from page.locals, which gets populated when executing the script. The decorators are also only defined when you execute the script. You could have the decorator populate a dictionary of hooks, which the page loader reads and then clears on each script execution. Yes, it's not threadsafe, but I believe we already have locks in place such that the script is only executed by one thread at a time (and probably isn't our biggest worry in terms of multiple threads executing the same script).


I'm also curious: in which situations would two pages be looking at the same model, but have different hooks? How could you ensure that they're both the same network if the code is different? Are we just going by file path, and allowing there to be two different live copies of the same file path? Why are we allowing a script to be associated with a particular path, but not be the same as what is present on disk?

@tcstewar
Copy link
Collaborator Author

The ExecutionEnvironment already modifies a bunch of global state. So that could potentially be used to either keep track of the page currently executed or to keep track of hooks registered while the code is executing. Or things could be injected via code_locals into the exec function. Or we could implement the decorator as a noop and inspect the AST.

I was thinking of just using the ExecutionEnvironment, but those other things are also decent options. However, one big functionality problem is that none of those options will work with IPythonViz, since the Page doesn't even exist at the time the model is being defined. I don't see a way around that... :(

@tcstewar
Copy link
Collaborator Author

You could have the decorator populate a dictionary of hooks, which the page loader reads and then clears on each script execution.

Interesting.... but that also fails in IPythonViz if I re-execute the cell that generates the visualizer. :(

I'm also curious: in which situations would two pages be looking at the same model, but have different hooks?

That's not the case I'm worried about -- I don't think that's possible. I'm worried about two different pages showing two different models, but there being one global list of hooks. That's why the simple approach of just doing @nengo_gui.register_hook('on_step') won't work, as we need to register that hook just for this page. If we're running the script inside nengo_gui, then this is doable by just looking at the context created by the ExecutionEnvironment. But if we're running inside Jupyter Notebook, that ExecutionEnvironment just doesn't exist. Instead Jupyter Notebook inspects locals() to figure things out. If the hooks are just functions defined in locals() (as in the current implementation), then that works fine.

I'm going to keep brainstorming to see if I can come up with an approach that uses decorators but gives the same functionality as the current system in this PR. I do think we'll want something like this eventually in nengo_gui, but it's probably not a huge priority right now. I do expect some people to need this sort of functionality at Nengo Summer School and at Telluride, but it's fine just switching over to this other branch for those cases (that's what we did last year, in any case).

@tbekolay
Copy link
Member

I'm worried about two different pages showing two different models, but there being one global list of hooks

Ooh I gotcha, that makes sense :)

@arvoelke
Copy link
Contributor

arvoelke commented Apr 30, 2018

I thought on_step used to work with nengo_gui, but tried searching for it and it brought me here? I was using it last year. Do you happen to know what version I should revert back to, to get this hook working with Nengo dev master? Or should I check out this branch as is? I'm trying to get nengo_learning_display to work. Thanks!

Edit: I checked out this branch and my old code worked as desired. :)

@arvoelke
Copy link
Contributor

Advantages of this approach (that I can immediately think of):

Another advantage of @jgosmann approach is that it will fail-fast if using an old version of the GUI, or if the name of the hook is miss-typed, or if it is unsupported by the backend. Case in point: when using the current version of nengo_gui, it was ignoring my on_step hook, which made the issue harder to debug.

@tcstewar
Copy link
Collaborator Author

tcstewar commented Jun 9, 2019

I'm adding this as an experimental feature for now, but there has been a ton of really good discussion about much better approaches than the current minimal version we've done here. My goal is to at least have this simple version in master, and then develop a better API for it, as discussed in #957 and #965 . I also think this interacts with #942 in that having a good API for this sort of behind-the-scenes interaction between code and the GUI seems important for a lot of things.

@simondlevy
Copy link

Thanks for adding this feature, Terry. It enabled me to use the GUI with the PID controller we worked on last year, to do a real-time simulation.

@zotric
Copy link

zotric commented Dec 14, 2023

Thank you, Terry. This solved a number of general issues with code being executed before starting the sim!
For my robotics application it has worked exactly as you planned.
New suggestion: To allow the Nengo gui application to be closed tidily when the browser window is closed but not when the gui reset button is clicked. At the moment both actions trigger on_close(sim).
I think this might be achieved if the reset button were to trigger a new on_reset(sim) event.
Then on_close(sim) would be the signal for the application to shut itself down tidily.
There might be a workaround for the application, involving detecting a double click, but a new event would be a lot tidier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants