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

Simulator Methods #44

Closed
voetberg opened this issue Mar 28, 2024 · 5 comments · Fixed by #43
Closed

Simulator Methods #44

voetberg opened this issue Mar 28, 2024 · 5 comments · Fixed by #43
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@voetberg
Copy link
Contributor

We need to allow users to bring their own simulator without requiring them to run the client outside of cli/config style.

I see two main approaches:

  • Use something like dill and ask users to pickle their sim with a defined io (horribly dangerous, basically just arbitrary code execution, but does exactly what we need with minimal user overhead)
  • Ask the user to follow a format (via an abstract class) and register it - the rucio plugin algorithm approach or rllib env approach. Which is more secure and probably less error prone, but harder upfront for the user.

I'm biased towards option 2, which is probably obvious.

@voetberg voetberg self-assigned this Mar 28, 2024
@voetberg voetberg added enhancement New feature or request help wanted Extra attention is needed labels Mar 28, 2024
@bnord
Copy link
Contributor

bnord commented Mar 28, 2024

Could you sketch out what we'd be asking the user to do, and how much time and effort that would take on their part?

I think this is the only variable I'm considering. E.g., if Registering is potentially very onerous for people, then it could be the swaying factor. If it's easy enough, then I'd agree to go with that.

@voetberg
Copy link
Contributor Author

Sure:

Option 1 -

We're asking people to write a function that can be pickled (e.g. is serializable), and save that and pass the path to the executable to the client code within the config or cli args. Besides the trick with it being serializable, very straightforward and we can check that people have gotten it right first thing by just loading it in. Their part looks like this:

def sim(thetas): 
       .... 
dill (/pickle/marshal/whatever).dump(sim, out_path)

and then adding a field to the config or an arg to the cli command that points to out_path.

Option 2 -

We write an abstract class (BaseSimulator or something similar) with the specific methods we need (basically just __call__) and ask them to run a method to register it with some internal part of code. (We can supply a command). Their part looks like this:

from DeepDiagnostics.simulator import Simulator, register_simulator

class MySim(Simulator): 
    def __init__(self): 
         .....
    def __call__(self, thetas): 
         .....

register_simulator("MySim", MySim)

And then adding the name of the simulator ("MySim") to the config.

@bnord
Copy link
Contributor

bnord commented Mar 28, 2024

Option 2 seems very close to people writing their own class for a neural network in pytorch. Is that correct?

@voetberg
Copy link
Contributor Author

Yep, very similar

@bnord
Copy link
Contributor

bnord commented Mar 29, 2024

I propose that we go with option 2 and we can get feedback internally from our crew in mid-April? if people balk at it, maybe we come up with a different strategy?

@voetberg voetberg linked a pull request Mar 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants