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

Firingrate norm for BVCs should be unscaled #108

Closed
colleenjg opened this issue Mar 21, 2024 · 2 comments · Fixed by #109
Closed

Firingrate norm for BVCs should be unscaled #108

colleenjg opened this issue Mar 21, 2024 · 2 comments · Fixed by #109

Comments

@colleenjg
Copy link
Contributor

I noticed that the BVC firingrates weren't scaling correctly if max_fr is not 1. I've traced the source of the issue to self.cell_fr_norm. This value is initialized based on scaled firingrates, but then applied before scaling when calling get_state(), and thus cancels out the subsequent scaling. See L1543 and L1679 of Neurons.py

import numpy as np

from ratinabox import Agent, Environment
from ratinabox.Neurons import BoundaryVectorCells

MAX_FR = 1

np.random.seed(10)

env = Environment()
ag = Agent(env)
BVCs = BoundaryVectorCells(ag, params={"min_fr": 0, "max_fr": MAX_FR})

for i in range (500):
    ag.update()
    BVCs.update()

min = np.asarray(BVCs.history["firingrate"]).min()
max = np.asarray(BVCs.history["firingrate"]).max()

print(f"{min:.4f} to {max:.4f}")

gives 0.0000 to 1.0165 (FYI: you can see there's a slight overshoot of the max, which might be for another issue.)

But, if you set MAX_FR = 10, you still get 0.0000 to 1.0165.

I'll create a PR in a moment to propose a solution.

@colleenjg
Copy link
Contributor Author

Minor edit: I hadn't pulled the latest version. The error is still present, but the exact values for both MAX_FR values are 0.0000 to 1.0088.

@TomGeorge1234
Copy link
Collaborator

Oh yes I see that is a bit of a bug, it totally cancels itself out. Not sure how I'd missed that/when it crept in. I'll take a look at your PR and leave this open until it's solved

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

Successfully merging a pull request may close this issue.

2 participants