Grid cell initialization does not preserve "n" provided in params dict if you provide low and high for distribution as a list

[jquinnlee]

Hey Tom,

I have been playing with the new GridCells class with the 1.7.0 update and was having issues with the number of grid cells not matching the number I provided at initialization. For example, when I initialized with the following params I would get 2 grid cells, and not 500 as I was hoping:

GC = GridCells(Ag, params={"n": 500, "gridscale": [0.28, 0.73], "gridscale_distribution": "logarithmic", "description": "three_shifted_cosines", "min_fr": 0, "max_fr": 1, "name": "GridCells")

I was expecting this to work since in the initialization the logarithmic option is supposed to receive "low" and "high" from a list provided in gridscales, which I think would be good to actually provide for all the distribution options that are sampling between some floor and ceiling value (uniform for example). The issue I think stems from this line during initialization: https://github.com/TomGeorge1234/RatInABox/blob/0ab2e66ea94cfb9817b41684f478d43e440bf39d/ratinabox/Neurons.py#L922

Whenever you want to provide a high and low, rather than a single value "n" will be overwritten as 2. I have some changes I've made to circumvent the issue and can make a pull request to contribute the change if you'd like!

Best, Quinn

[TomGeorge1234]

oh....yeah my mistake I wrote that logarithmic feature and then backtracked but clearly forgot to take it out. The reason is that params['gridscales'] can ALSO be just an array. In which case RatInABox assumes this array is one grid scale for each grid cell and overwrites params['n']...so hence why you got two gridcells with grid scales 0.28 and 0.73.

Not sure the best way round this. A quick fix is definitely just to make your own grid scales and pass them in manually as an array, something like params['gridscales'] = np.logspace(0.1,1,500)

I'll have a think and probably try push a fix next week.

[jquinnlee]

Gotcha, that makes total sense. Manually passing the grid scales in this way is straight-forward, and I will do that in the mean time.

One potentially reasonable assumption is that if someone has generated a thrown in a distribution manually it will be more than two values (e.g. not for just two grids). In this case, you could add to the previously mentioned line:

if hasattr(self.params["gridscale"], "__len__") and len(self.params["gridscale") > 2:

Maybe it is too risky, but it's a possible tweak that would be simple.

[TomGeorge1234]

Cool. Pushed a fix. Now if params['gridscale'] is an array or list it's taken to be manually setting the neurons. If it's a tuple it's assumed to be parameters of the distribution in question. Think this is the cleanest way to do it. So this should work now:

GCs = GridCells(params={
"gridscale": (0.28, 0.73,),
"gridscale_distribution": "logarithmic"
})

but notice the change from a list to a tuple.

Btw I've defined a function in utils called distribution_sampler() which actually stores all the options (normal, rayleigh, delta, logarithmic etc.) as it was getting messy repeating code all over the shop. Fun fact, theres a modules option to get evenly sized modules of grid cells. E.g. try:

GCs = GridCells(params={
"gridscale": (0.1,0.4,1.0),
"gridscale_distribution": "modules",
})

Could you check this is working for you? Then I'll close the issue.

[jquinnlee]

Hey Tom,

Sorry for the delay on this. I can confirm the fix works perfectly! Thanks for making that change. The module option is very interesting. I will incorporate this into the GC-PC options we have discussed once I have some more time to code that up (hopefully in the next week or two). btw I have a version of the Solstad model working pretty well on the fits to real data :) Will make a PR for that soon I hope.

All the best, Q

[TomGeorge1234]

Great looking froward to seeing it !