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

GUI: Change GUI mentions of Layer 2 (L2) to Layer 2/3 #904

Closed
asoplata opened this issue Oct 15, 2024 · 2 comments
Closed

GUI: Change GUI mentions of Layer 2 (L2) to Layer 2/3 #904

asoplata opened this issue Oct 15, 2024 · 2 comments
Assignees
Labels
hnn-gui HNN GUI

Comments

@asoplata
Copy link
Collaborator

Problem:

Currently, the hnn-core GUI only displays layer information for either "Layer 2" or "Layer 5". However, it is more scientifically accurate to use the term "Layer 2/3" instead of "Layer 2", since the former is closer to what we are simulating.

Solution:

I think the original intention was to change literally where the GUI displays things like layer2 such as here ( https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/gui/_viz_manager.py#L96 ) and, similarly, here ( https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/viz.py#L330 ). This is trivial and I will have no problem changing similar things.

However...

Greater Context:

This would also introduce a small amount of both visual and naming inconsistency. As (crudely) highlighted in the below screenshot, updating only the GUI plot titles for display would change the plot title on the right, but anything shown that pulls from a Network object or the canonical cell_types, such as on the left, is still going to show L2.

Screenshot_20241015_165945

To manage this inconsistency, I can only think of three possibilities:

  1. We only change the GUI plot titles / radio button names, but leave any auto-loaded cell_type names alone. We then add a note / clarification in the documents that says we use L2 etc., in the code for historical reasons, but that any time we say "layer 2" in any form, we really scientifically mean "layer 2 / 3". This is basically the same as what I said in the Solution above (assuming we don't already have a clarification that says that). Additionally, we could even have text warnings displayed on the GUI where relevant, like "Anything indicated by L2 is equivalent to Layers 2 / 3", though that feels hacky. Most domain experts using HNN will NOT be confused by the L2 / L23 naming, especially when we don't have separate L3 celltypes.
  2. We change ALL representations of layer 2 to become layer 2/3 (so L2->L23, etc.), in not only the GUI, but also the code, and also the param files. This would make everything consistent, however it would break backwards-compatibility for the param files! Older representations of the same Jones 2009 network like https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/param/default.json would have to be converted, any external code that relies on L2Pyr_... parameters will break, any code that parses the layer name would have to be checked, etc. I think this is a bad idea, but still wanted to bring it up just in case.
  3. (We could also override the loaded names of the cell_types in the GUI but not in the actual data, but this is a terrible idea).

I suggest we go with Option 1 above (minor changes only), unless someone wants to seriously discuss Option 2.

@asoplata asoplata added the hnn-gui HNN GUI label Oct 15, 2024
@asoplata asoplata self-assigned this Oct 15, 2024
asoplata added a commit to asoplata/hnn-core that referenced this issue Oct 18, 2024
Note that this does not change the L2 names inherent in the `cell_types`
which are loaded dynamically in the gui, such as shown here:
jonescompneurolab#904 (comment)
@dylansdaniels
Copy link
Collaborator

dylansdaniels commented Oct 21, 2024

@asoplata pasting here as requested :)

the cell properties widget output: gui.app_layout.left_sidebar.children[0].children[0].children[1].children[1].children[1]

you can use the .get_state() method and look at the 'outputs'; e.g.,

{'output_type': 'display_data',
  'data': {'text/plain': "VBox(children=(BoundedFloatText(value=0.01, description='Soma Kv channel density (S/cm2)', layout=Layout(width…",
   'application/vnd.jupyter.widget-view+json': {'version_major': 2,
    'version_minor': 0,
    'model_id': 'b1c383b0585f49ef9c7a97f45a678e02'}},
  'metadata': {}},
 {'output_type': 'display_data',
  'data': {'text/plain': "VBox(children=(BoundedFloatText(value=0.01, description='Soma Kv channel density (S/cm2)', layout=Layout(width…",
   'application/vnd.jupyter.widget-view+json': {'version_major': 2,
    'version_minor': 0,
    'model_id': 'b1c383b0585f49ef9c7a97f45a678e02'}},
  'metadata': {}}

asoplata added a commit to asoplata/hnn-core that referenced this issue Oct 21, 2024
Note that this does not change the L2 names inherent in the `cell_types`
which are loaded dynamically in the gui, such as shown here:
jonescompneurolab#904 (comment)
asoplata added a commit to asoplata/hnn-core that referenced this issue Oct 21, 2024
Note that this does not change the L2 names inherent in the `cell_types`
which are loaded dynamically in the gui, such as shown here:
jonescompneurolab#904 (comment)
gtdang pushed a commit that referenced this issue Oct 22, 2024
Note that this does not change the L2 names inherent in the `cell_types`
which are loaded dynamically in the gui, such as shown here:
#904 (comment)
@asoplata
Copy link
Collaborator Author

Resolved from #907

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hnn-gui HNN GUI
Projects
None yet
Development

No branches or pull requests

2 participants