[WIP] Experimental duecker_ET model branch #1168
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
@ntolley yay! I made a small change this week which requires another round of optimization, which is a great chance to use Carolina's new initial params feature :) I will provide some guidance on how to use the model once that is done. |
asoplata
added a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 6, 2025
This is the first commit of many to come where I'm going to extract the parts of jonescompneurolab#1168 relevant to the new, proper voltage initialization. The plan is: 1. We'll separate the changes to the "v_init" stuff and make it its own PR onto `master`, 2. Change the tests that become broken as we go along, 3. Everyone reviews the changes (which will likely end up in a slightly different form from how they currently exist in jonescompneurolab#1168), 4. We merge these voltage initialization changes into `master`, 5. Then finally, we merge/rebase the voltage initialization changes (which are now on `master`) *back* into the code at jonescompneurolab#1168 . 6. Repeat the process for all code features included in the length jonescompneurolab#1168 work, until everything gets merged, piece-by-piece. For starters, this commit consists of *only* the extracted changes to `cell.py` and nothing else. Fortunately, in this case, the tests seem to pass without having to change anything.
asoplata
added a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 6, 2025
This fixes what I think is a bug currently present in jonescompneurolab#1168, where the L2's `soma` section did not have its universal voltage-initialization value applied (but the dendrites did). Additionally, @katduecker , I think the same bug is present in your `_get_interneuron_soma()` function (which I've removed in this commit since it's not relevant to the voltage-initialization). Your `_get_interneuron_soma` takes a `v_init` value, but does not apply it in `Section` creation. This was probably missed. This also removes `NetworkBuilder.state_init()` like KD did, since it is no longer needed, as the voltage initialization is completely moved over to `cells_default.py` now. With this, I think the voltage-initialization stuff has been completely extracted from jonescompneurolab#1168. The only remaining work is to write new tests specifically for the `v_init` where it is used.
asoplata
added a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
This fixes what I think is a bug currently present in jonescompneurolab#1168, where the L2's `soma` section did not have its universal voltage-initialization value applied (but the dendrites did). Additionally, @katduecker , I think the same bug is present in your `_get_interneuron_soma()` function (which I've removed in this commit since it's not relevant to the voltage-initialization). Your `_get_interneuron_soma` takes a `v_init` value, but does not apply it in `Section` creation. This was probably missed. This also removes `NetworkBuilder.state_init()` like KD did, since it is no longer needed, as the voltage initialization is completely moved over to `cells_default.py` now. With this, I think the voltage-initialization stuff has been completely extracted from jonescompneurolab#1168. The only remaining work is to write new tests specifically for the `v_init` where it is used.
asoplata
pushed a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
This is the first commit of many to come where I'm going to extract the parts of jonescompneurolab#1168 relevant to the new, proper voltage initialization. The plan is: 1. We'll separate the changes to the "v_init" stuff and make it its own PR onto `master`, 2. Change the tests that become broken as we go along, 3. Everyone reviews the changes (which will likely end up in a slightly different form from how they currently exist in jonescompneurolab#1168), 4. We merge these voltage initialization changes into `master`, 5. Then finally, we merge/rebase the voltage initialization changes (which are now on `master`) *back* into the code at jonescompneurolab#1168 . 6. Repeat the process for all code features included in the length jonescompneurolab#1168 work, until everything gets merged, piece-by-piece. For starters, this commit consists of *only* the extracted changes to `cell.py` and nothing else. Fortunately, in this case, the tests seem to pass without having to change anything. The original person who actually wrote this code was @katduecker, which is why they are indicated as author. This version of the code that was extracted from jonescompneurolab#1168 was done so by @asoplata .
asoplata
pushed a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
This does NOT pass tests The original person who actually wrote this code was @katduecker, which is why they are indicated as author. This version of the code that was extracted from jonescompneurolab#1168 was done so by @asoplata .
asoplata
pushed a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
This also includes regenerated networks, since doing that is necessary to allow the tests to pass as well. The original person who actually wrote this code was @katduecker, which is why they are indicated as author. This version of the code that was extracted from jonescompneurolab#1168 was done so by @asoplata .
asoplata
added a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
This fixes what I think is a bug currently present in jonescompneurolab#1168, where the L2's `soma` section did not have its universal voltage-initialization value applied (but the dendrites did). Additionally, @katduecker , I think the same bug is present in your `_get_interneuron_soma()` function (which I've removed in this commit since it's not relevant to the voltage-initialization). Your `_get_interneuron_soma` takes a `v_init` value, but does not apply it in `Section` creation. This was probably missed. This also removes `NetworkBuilder.state_init()` like KD did, since it is no longer needed, as the voltage initialization is completely moved over to `cells_default.py` now. With this, I think the voltage-initialization stuff has been completely extracted from jonescompneurolab#1168. The only remaining work is to write new tests specifically for the `v_init` where it is used.
asoplata
reviewed
Nov 7, 2025
| # each trial needs unique event time vectors | ||
| for trial_idx in range(n_trials): | ||
| for drive in self.external_drives.values(): | ||
| for d, drive in enumerate(self.external_drives.values()): |
Collaborator
There was a problem hiding this comment.
Is there a not-yet-included reason why this enumerate was included? I don't believe it's used currently.
| """ | ||
|
|
||
| return zero_val * np.exp(exp_term * x) + offset | ||
| return zero_val * (slope * np.exp(exp_term * x) + offset) |
Collaborator
There was a problem hiding this comment.
There may be a bug here: offset is now multiplied by zero_val, but it wasn't before. In the current master code, offset is always 0, so this change does not break current tests. However, that doesn't mean it's not a bug; @katduecker are you sure that offset should be multiplied by zero_val? I can understand that that may be your very reason for introducing slope, but just wanted to double-check.
Collaborator
Author
There was a problem hiding this comment.
Yes, this is what I want! The function is taken from Hay et al., 2011
gbar = gsoma * (-0.8696 + 2.087 * np.exp(x/323))
asoplata
pushed a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 7, 2025
I believe this adds the last of the code that is extracted from jonescompneurolab#1168 that is *not* the actual Duecker model itself, but instead code changes that are used by the default model. These functions are not currently tested at all, and I have not added tests for them for similar reasons to jonescompneurolab#1185 (comment) After this, we should be able to make a PR that adds the actual Duecker model itself. However, there will be additional work for that step: we need to update `hnn_io.read_network_configuration` and the equivalent `...write...` such that they support the Duecker model. Continuing the pattern, this is based on top of jonescompneurolab#1187.
asoplata
pushed a commit
to asoplata/hnn-core
that referenced
this pull request
Nov 12, 2025
I believe this adds the last of the code that is extracted from jonescompneurolab#1168 that is *not* the actual Duecker model itself, but instead code changes that are used by the default model. These functions are not currently tested at all, and I have not added tests for them for similar reasons to jonescompneurolab#1185 (comment) After this, we should be able to make a PR that adds the actual Duecker model itself. However, there will be additional work for that step: we need to update `hnn_io.read_network_configuration` and the equivalent `...write...` such that they support the Duecker model. Continuing the pattern, this is based on top of jonescompneurolab#1187.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the latest version of the duecker_2025_ET_model, rebased to the current state of the branch. The model is open to all HNN developers at their own risk and discretion. This model is still under development, not peer-reviewed, and regular code changes are expected.
I will add guidelines on how to use the model to this PR. Please reach out to me if you have any questions!