Updating the trainable_params variable when write_trainables method

[Kartik-Sama]

When the write_trainables function is invoked, the parameters of the module are being changed in the lines in the write_trainables method.

However when the method get_parameters is called (which is supposed to return only the trainable parameters) the class member self.trainable_params is being accessed. Which is populated in the make_trainable method, but is never updated in the flow of the write_trainables method.

The code changes address Issue 709 - it updates the stale values being returned by get_parameters method when changes to trainable parameters are made through the write_trainables method. This done by maintaining the correct state of trainable parameters in the class member self.trainable_params.

[michaeldeistler]

Thanks Kartik! I think the more elegant solution would be to delete the self.trainable_params attribute and move this code to def get_parameters(). This would also allow things like:

cell.make_trainable("gNa")
cell.set("gNa", 2.0)
params = cell.get_parameters()

and it would return the updated params from get_parameters. Does this make sense? IIUC, then this would also fix your issue, no?

I might be missing something though, so if you see a reason for why this does not work, then let me know!

[Kartik-Sama]

I agree this is a hacky solution and having the logic moved to get_parameters would be elegant.

I checked the code block you have pointed out to it seems the code under init_val condition can be dropped (since we don't initialize anything when we fetch existing values of trainables) and lines 1455 and 1474 should suffice while we still keep track of the trainable parameter names. But I will think more carefully about it and get back to update the PR!

[Kartik-Sama]

I have updated the PR for get_parameters to now fetch the trainable parameters values from the dataframe associated with the base module itself, where the parameter values are being updated in write_trainables or the set method.

However, the PR is still incomplete. I have few doubts,

• I am not sure how to remove the self.trainable_params member primarily because I don't understand how the partial and completely in view indices work here, and what their use cases could be. So I have refrained from removing self.trainable_params because their role/implications in Views is still unclear for me.
• In the make_trainable method if the init_values are supplied I would rather call the write_trainables method with the corresponding init values to make the code flow clearer and more predictable.

[michaeldeistler]

Hi Kartik,

I am not sure how to remove the self.trainable_params member primarily because I don't understand how the partial and completely in view indices work here...

You can remove these bits of code.

In the make_trainable method if the init_values

I think we should just get rid of the option to provide an init_value to make_trainable. @kyralianaka would you be ok with that? Are you using this? I.e. something like:

net.make_trainable("HH_gNa", 0.2)

[kyralianaka]

Hello there, yes I initialize trainable parameters using make_trainable() currently everywhere, and I think it's a major API change that could at least go to the v1 release. Maybe there is a way to still delete self.trainable_params but rather call set() (underneath) if initial values are provided?

[michaeldeistler]

Okay, then let us not remove the init_value option.

However, maybe the easiest option is the one that @Kartik-Sama had originally implemented: to just write to self.trainable_params in write_trainables(). @Kartik-Sama do you see any major downsides to this?

[Kartik-Sama]

I see that trainable parameters values are being stored in two places 1) dataframe corresponding to the module's base 2) self.trainable_params

Having both of them is fine as long as both these places are consistently updated whenever changes happen. But the cleaner way would be to just have one place to store this information and retrieve it in the format necessary. We can still get rid of trainable_params member while having the init in make_trainable though. Directly updating the code in write_trainable is an easy fix, but still looks like a shaky fix to me.

I also have doubts with a test though. Here the params are being fetched from get_params twice (at lines 98 and 120). However in between these lines I see no writing or setting of trainable_params variable, yet this assertion checks for the new value of the parameter without being actually set. I am definitely missing something here, but the test itself isn't very clear to me.

[michaeldeistler]

We can still get rid of trainable_params member while having the init in make_trainable though

How? Where will the trainable_params be stored?

[Kartik-Sama]

trainable_params can be split into two chunks of information.

1. The names of trainable parameters
2. Indices where you have to look up the parameter values (which is already being stored in indices_set_by_trainables)

In that way we can always call get_parameters to load the current state of trainable parameters, since we know the column and the indices (and thereby using .loc) to retrieve or set their values.

For instance the write_trainables works without ever setting the trainable_params because it is updating the underlying data frame correctly. Thus, we can just convert the init_val to appropriate format and just call write_trainables method from inside make_trainable method. (Unless I am missing something)