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

Access to built-in "uniform" function for PlaceCells / Agent velocity initialization #5

Closed
PikaPei opened this issue Oct 26, 2022 · 11 comments

Comments

@PikaPei
Copy link

PikaPei commented Oct 26, 2022

Great work, it's simple and pretty!

I'm trying the 1D version. Here are some small questions/suggestions.

In the class PlaceCells, place_cell_centres uses "uniform_jitter" method to sample the position.
Although I can pass an array into place_cell_centres, I think it's easier to use the built-in "uniform" method?

When initializing Agent, if I didn't use the argument params but changed speed_mean directly, the initial velocity wouldn't be updated (it's still the default value).
However, if using the params, it's correct. Not sure if this would be a problem or not.

Ag.speed_mean = 0.20
# Ag.velocity = 0.08

Ag = Agent(Env, params={"speed_mean": 0.20})
# Ag.velocity = 0.20
@TomGeorge1234
Copy link
Collaborator

TomGeorge1234 commented Oct 26, 2022

Thank you! And thank you for raising this issue.

  1. I see. Yes, you have understood this correctly. I assume you would like evenly spaced place cells WITHOUT jitter. Currently the only way to do this is do sample these positions yourself (using Environment.sample_positions(method="uniform"), or just np.linspace()) and pass this in at initialisation. Ultimately there are many examples where it would be nice to control things like this at initialisation but this comes at the expense of complexity and elegance which was why I didn't include it. However I'm happy for you to add it. Feel free to edit the class and open a pull request: I would suggest the most elegant solution would be that the place_cell_centres parameter could be used to hand in a string "uniform" or "uniform_jitter" controlling the argument method which is passed to sample_positions(method=self.place_cell_centres) on line 554.

  2. Yes, this is intended behaviour. The Ag.speed_mean attribute just sets the mean speed towards which the stochastic process will cause the speed to drift, not the speed itself. So changing it after initialisation will not immediately change the actual speed of the Agent, as you observe, but after a short period of exploration you will find the velocity varying around the new mean of 0.2. On the other hand, at initialisation the velocity is set to the speed_mean parameter, hence why this change is visible immediately. Ultimately, both will work and will produce very similar data where the Agents mean speed sits around 0.2.

@PikaPei
Copy link
Author

PikaPei commented Oct 27, 2022

Thank you for the explanation!

  1. Now I know why you didn't include these methods. And thanks for the suggestion, I'll try to make a pull request! (although I personally would like to use the built-in method, I don't know if it's necessary for this package or not 😅)

  2. Ok, I see! I totally forgot I can set Ag.velocity directly...


After updating the package, the 1D version raised an error.

Env = Environment(params={"dimensionality": "1D"})
Ag = Agent(Env)
PCs = PlaceCells(Ag)
Warning you have solid 1D boundary conditions and non-zero speed mean. 
Traceback (most recent call last):
  File "/home/hsulab/project/RatInABox/demos/test.py", line 10, in <module>
    PCs = PlaceCells(Ag)
  File "/home/hsulab/.local/lib/python3.10/site-packages/ratinabox/Neurons.py", line 574, in __init__
    len(self.Agent.Environment.walls) > 5
AttributeError: 'Environment' object has no attribute 'walls'

It seems that it didn't detect the 1D environment?

@TomGeorge1234
Copy link
Collaborator

I'm happy for the change to be made. I agree it might be restrictive to enforce jitter on the place cell position. I leave it up to you if you want to PR it.

Sorry, that's a trivial bug I knew about. I think I've fixed it now, please can you confirm if it is working.

@PikaPei
Copy link
Author

PikaPei commented Oct 27, 2022

Yes, it works well now! Thanks for the quick update.
I've also made a PR about place_cell_centres.

@TomGeorge1234
Copy link
Collaborator

Glad it's working! I'll close this issue.

@TomGeorge1234
Copy link
Collaborator

TomGeorge1234 commented Nov 7, 2022 via email

@PikaPei
Copy link
Author

PikaPei commented Nov 9, 2022 via email

@TomGeorge1234
Copy link
Collaborator

TomGeorge1234 commented Nov 9, 2022 via email

@PikaPei
Copy link
Author

PikaPei commented Nov 29, 2022 via email

@TomGeorge1234
Copy link
Collaborator

TomGeorge1234 commented Nov 29, 2022 via email

@PikaPei
Copy link
Author

PikaPei commented Nov 30, 2022 via email

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

No branches or pull requests

2 participants