-
Notifications
You must be signed in to change notification settings - Fork 221
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
Pragmatic solution to parameter changes in standalone mode #1429
Conversation
If this works it is already a lot better than what I did in the towards encapsulation branch which is hopelessly out of date and presumably no longer rescuable. We probably want to think about the syntax a bit, but that's fine. There's also the question of when these command line arguments get run which might introduce some issues? There's still no solution I think to the problem of making number of neurons a variable that you can change, right? I think that needs a more fundamental change. But I think this is worth doing. We want to have this functionality and if we wait for the perfect solution we'll never have it. |
I'm away for August so if we want to discuss syntax, can we do that tomorrow/Friday? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Currently, they set the variables just before the
You are right, this would be a more fundamental change and is not supported with the solution I present here. We hardcode constants like this as unchangeable all over the code, so I don't see any quick solution.
Happy to discuss here (or over video or something) any time this afternoon or tomorrow. But I will be away for August as well, so it won't change much 😊 |
We could have a bit of syntax that allows you to choose where to set the values. Allows more flexibility because then the code can do some derived values from that, e.g.
Let's discuss more in September if you're also away in August. |
Yes, worth discussing! I wonder whether it is worth the complication, given that in most cases you could do the same by simply setting dependencies explicitly, e.g. in your example run(tau_exc=5*ms, tau_inh=50*ms) A lightweight solution could also be to include this information in the
Sounds good, let's find a time over email. |
RTD runs Python 3.7, recent sympy versions don't support it
@thesamovar I think this is more or less ready to go – not quite mergeable yet, since the C++ code is still a bit convoluted, but let's make sure that we agree on syntax before I finalize things. This now supports all the use cases I mentioned above: repeatedly running a simulation to get different random values, setting variables to scalars, and setting variables with arrays. Together with the new option to choose the results directory, this can also be used with several simulations running in parallel. The current syntax does not use direct keyword arguments to As I argued earlier, I like such a general mechanism that allows changing parameters without needing to change in the existing source code (e.g. for I triggered a documentation build for this branch for easier reading of the new docs. |
It'll be a little while before I have time to do this unfortunately. But it's going on my priority to-do list. |
No worries. I've also used the opportunity to ask for feedback on the discussion forum. |
# Conflicts: # brian2/codegen/codeobject.py # brian2/devices/cpp_standalone/device.py # brian2/tests/test_cpp_standalone.py # rtd-requirements.txt
Also add a test for setting synaptic variables
It's been a while (almost a year!). This is now ready-to-merge from my side, I'm quite happy with it and already started using it myself (in particular for |
I am going to ignore our usual procedure here, and will merge this now as it is without waiting for review (which would probably take a while to come in with summer holidays and all that). This is a major addition, but I am quite happy with it, and I'd say it is fairly well tested and documented. One important reason for merging it is also that I'd like to get users to use/test this feature, and that is easier if they can install the package from testpypi. But it also simply becomes hard to keep such long-running PRs updated, adding significant work. Of course, the fact that I merge it now doesn't mean that you shouldn't have a closer look at it, please do! Let's consider this making me the case for post-publication review (or EAFP) 😉 |
We have been discussing for a long time how we can make things like parameter explorations more convenient with standalone mode (see discussions about "encapsulation mode" #251). I recently thought about this a bit more, and came up with an intermediate solution that I'm really happy about. It has the advantage that 1) it already works with this PR and 2) it requires minimal changes to code and devices like
brian2cuda
. The basic idea is:main
binary to set variable values. This value can either be a scalar, or the name of a file containing the values (which makes it possible to specify whole arrays, which otherwise wouldn't be really convenient via the command line).results
directory configurable by command line argumentsThe difference of 1) with what we discussed earlier (#251) is that I enabled this for arbitrary variables of the model – this removes the whole need for a discussion of syntax to define which variables should be changeable by a user (
Parameter
class, etc.). The addition of 2) makes it immediately possible to use this for parallel simulations. The syntax indevice.run
(see examples below) still needs to be improved, transforming the initialization into command line arguments is of course not something that the user should do. Oh, and don't look too closely at the C++ code for now :)The main thing that is missing functionality-wise for now is the ability to set arbitrary constants defined outside of equations. Your
towards_encapsulation
branch already had a change where instead of "freezing" the constants, it would put them as variables into a file. Applying this change here would make implementing this functionality almost trivial, but I'd like to leave this out of the very first version of this feature, since it requires bigger changes in devices likebrian2cuda
– you can work around this limitation by including a variable liketau : second (shared, constant)
in your model and setting this instead of an external constant.Examples
Here are a few things that are possible with these changes:
Run a simulation several times to get new random number instantiations, without recompiling the code (this was already possible before, just mentioning it here for completeness):
To run this in parallel, you'd previously had to change the project directory for each process, which meant that each process would compile everything again. With this PR, you can compile it once and then run it in different result directories (#1240):
And now the main change: setting an initial value via the command line. This will set the
tau
value (now included in the model as discussed above):And by going via a file, you can set the initial values of
v
to whatever you like:The latter examples could also run things in parallel by combining this with the
results_directory
argument.Again, not claiming that we should keep the
run_args
syntax (or make the user store the values to a file), but the great thing about this is that it already works and that it will work with minimal changes forbrian2cuda
,brian2genn
, etc., since they leave the loading/saving values from/to disk to Brian. It should also be noted that this actually already provides a C++ API to change parameters for a run—admittedly an ugly one: you could linkmain.cpp
and call something likemain(2, {"mycall", "neurongroup.tau=0.05"});
from your C++ code...I feel like this feature (with a syntax that replaces setting
run_args
manually) would make a lot of people happy, and is actually feasible with minimal effort. What do you think, @thesamovar ?