add_agc_plus_receptor_potential #21
Open
Conversation
This file contains 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
|
@robsc looks a little better...hope its suitable for review!! |
robsc
reviewed
Dec 30, 2024
python/jax/carfac.py
Outdated
| @@ -2483,10 +2493,12 @@ def run_segment_jit( | |||
| naps: neural activity pattern | |||
| naps_fibers: neural activity of the different fiber types | |||
| (only populated with non-zeros when ihc_style equals "two_cap_with_syn") | |||
| receptor_pot: receptor potential of ihc | |||
| state: the updated state of the CARFAC model. | |||
| BM: The basilar membrane motion | |||
| seg_ohc & seg_agc are optional extra outputs useful for seeing what the | |||
There was a problem hiding this comment.
Not your change: but lets split this comment to really have one per arg.
There was a problem hiding this comment.
Thanks Rob, do you mean the seg_ohc + seg_agc?
python/jax/carfac.py
Outdated
| naps = jnp.zeros((n_samp, n_ch, n_ears)) # allocate space for result | ||
| naps_fibers = jnp.zeros((n_samp, n_ch, n_fibertypes, n_ears)) | ||
| receptor_pot = jnp.zeros((n_samp, n_ch, n_ears)) |
There was a problem hiding this comment.
for the receptor pot, is it more of an optional item, a la agc and agc_memory?
if so, name as such and place at end of outputs.
There was a problem hiding this comment.
Yup its technically optional and more for debugging + further changes...will move it around
python/jax/carfac_bench.py
Outdated
| @@ -179,7 +179,7 @@ def loss_func( | |||
| weights: carfac_jax.CarfacWeights, | |||
| state: carfac_jax.CarfacState, | |||
| ): | |||
| nap_output, _, _, _, _, _ = carfac_jax.run_segment( | |||
| nap_output, naps_fibers_output, _, _, _, _ = carfac_jax.run_segment( | |||
There was a problem hiding this comment.
Why add this as a named output (here and below)
There was a problem hiding this comment.
Not necessary at all...probably left over from testing!!
Made changes as requested + altered other calls to run_segment/run_segment_jit that accompany changes to carfac.py
receptor_pot (ihc receptor potential in two_cap model) is now an optional extra and positioned accordingly. Also separated + clarified origins of seg_ohc + seg_agc
carfac_float64_test + carfac_util to match changes to run_segment in carfac.py
No matches carfac.py changes to receptor_pot
|
@robsc Hope these changes are suitable...I have another question...I use a for loop to assign agc_memory to the final output...do you think this be performed by concatenating agc_memory across state.ears[ear].agc[:].agc_memory and then assigning? |
|
Only the stage 1 AGC output is needed, not collecting across agc[:], I
think.
Stage 1 already includes the summed outputs of the later stages (see book
Fig. 19.5).
Dick
…On Tue, Dec 31, 2024 at 2:14 PM JasonMH17 ***@***.***> wrote:
@robsc <https://github.com/robsc> Hope these changes are suitable...I
have another question...I use a for loop to assign agc_memory to the final
output...do you think this be performed by concatenating agc_memory across
state.ears[ear].agc[:].agc_memory and then assigning?
—
Reply to this email directly, view it on GitHub
<#21 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGW7DFRFOIAGEQT62EV2O32IID2HAVCNFSM6AAAAABUGUHEB2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRWGA4TMOJRGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Perhaps we should hold off this change then till we are all in person....originally I foresaw the AGC_memory as being accessible for debugging (rather like the CARFAC_hacking.m plots 4 AGC stages for every segment in matlab) therefore I thought that having all four stages might help if we plan to alter any temporal/spatial smoothing in the future...however happy to provide an example of the recalculated AGC from the Zb_memory parameter (even if it doesnt quite look the same...presumably due to the temporal smoothing/summation involved?!
Jason
…________________________________
From: Dick Lyon ***@***.***>
Sent: Tuesday, 31 December 2024 2:27 PM
To: google/carfac ***@***.***>
Cc: Jason Mikiel-Hunter ***@***.***>; Rob Schonberger ***@***.***>
Subject: Re: [google/carfac] add_agc_plus_receptor_potential (PR #21)
Only the stage 1 AGC output is needed, not collecting across agc[:], I think.
Stage 1 already includes the summed outputs of the later stages (see book Fig. 19.5).
Dick
On Tue, Dec 31, 2024 at 2:14 PM JasonMH17 ***@***.******@***.***>> wrote:
@robsc<https://github.com/robsc> Hope these changes are suitable...I have another question...I use a for loop to assign agc_memory to the final output...do you think this be performed by concatenating agc_memory across state.ears[ear].agc[:].agc_memory and then assigning?
—
Reply to this email directly, view it on GitHub<#21 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGW7DFRFOIAGEQT62EV2O32IID2HAVCNFSM6AAAAABUGUHEB2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRWGA4TMOJRGE>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
agc_memory_out has been removed and replaced with agc_out. agc_out now equals the stage 1 and final agc output
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updated carfac.py + test + utils/tests + bench + float64_test to include receptor potential + agc_memory as outputs