[MRG] make L5/L2 pyr have different geometry options in GUI #848
Conversation
|
@ntolley The code looks for variables in the |
|
@kmilo9999 the original code actually didn't have an L2 and L5 pyramidal cells differ in that L5_pyramidal cells have an What I'm trying to do is have the |
|
@ntolley I think the loops in the |
|
@kmilo9999 I agree we can revamp the logic in a followup PR In any case, I think I've updated the mappings that needed to be added and wrote some tests that they match what we get from making a network with the CLI Do you mind reviewing to make sure everything is correct? |
ntolley
changed the title
|
@ntolley This is what I got: Using UI cell params Not using the UI cell params (default values) Correct me if I am wrong. |
hnn_core/gui/gui.py
Outdated
| @@ -1669,7 +1701,12 @@ def _init_network_from_widgets(params, dt, tstop, single_simulation_data, | |||
| cell_type = vbox_key.split()[0] | |||
| update_function(single_simulation_data['net'], cell_type, | |||
| cell_param_list.children) | |||
| break | |||
| # break # why is this here? | |||
There was a problem hiding this comment.
@kmilo9999 let me know when you have time to look at this
There was a problem hiding this comment.
It's a small optimization.
Once the conditions are meet and a function is called, the loop doesnt need to continue to go through unnecessary iterations. This helps on correctness to guaranty no other calls to the same conditions are made.
| @@ -1963,7 +2031,7 @@ def update_common_dendrite_sections(sections, mechs_params): | |||
| name for name in sections.keys() if name != 'soma' | |||
| ] | |||
| for section in dendrite_sections: | |||
| sections[section].mechs.update(mechs_params) | |||
| sections[section].mechs.update(deepcopy(mechs_params)) | |||
There was a problem hiding this comment.
Is this deepcopy necessary ? This operations hit the performance of the app
There was a problem hiding this comment.
it is necessary because a nested dictionary is being added, I tried a regular .copy() and found that the inner dictionary was still pointing to the same variable
This is only necessary for the ar mechanism since gbar_ar is a partial function who's outputs depend on the geometry of each section
There was a problem hiding this comment.
I'll revisit this in a future PR. Because of my main logic you were force to use the deepcopy. I can rework it to avoid this call
|
Sounds great @kmilo9999! I'm good with the current changes so feel free to merge when you're ready. I have some ideas for variable name changes to increase clarity but we can work that out separately |
The current implementation of the cell parameter widget skips over a compartment of L5 pyramidal cells
This PR correctly updates the compartment parameters, as well as improves testing to make sure the GUI network matches the API instantiated network