Closed Tobias-Fischer closed 10 years ago
Looks good but I can't really evaluate it properly.
It would be great if you could add some unit tests to Imagen to make this works the way you think it should! Alternatively, it might be worth having a notebook to demonstrate Sweeper + multichannel retinas..
The notebook which I will provide soon will show sweeping of multichannel retinas. I confirmed that this is working a while ago. Unfortunately, I won't have time to write a unit test anytime soon (as vacation is coming up very soon :) ).
This PR is now updated and does not include the changes anymore which require rebuilding of the test data.
I think this will probably be the first PR that we will be able to merge.
Once merged, maybe the multi-channel components will prove a good enough reason to release a new imagen? See ioam/topographica#582
While we are discussing Sweeper
, I would also like to mention the proposed improvement described in issue #35.
Good to see this merged!
Tobias, are we going to have a PR for PatternCoordinator
? If so, there is one parameter 'period' that I would like to discuss with you.
Unfortunately, I won't have time for a PR for PatternCoordinator
. As I wrote somewhere in the topographica PR, I am not too sure which code should go where. The implementation itself should be straightforward and mostly copy+paste, but finding the right places for the bits and pieces might be tricky and some discussion will be involved. I don't see this merged within a day or two, but all that's what I have left before the vacation ;).
Can you explain what you mean here? Do you mean that you have code that's described in your thesis for PatternCoordinator, that isn't included in any of your pull requests? Oh dear! How will we get that?
I was referring to Jean-Luc's comment that he wants to see a PR for PatternCoordinator, so that the code marked with
# all the below will eventually end up in PatternCoordinator!
and
# all the above will eventually end up in PatternCoordinator!
in topo/submodel/earlyvision.py will be moved to imagen.
So the code is there, just at the wrong place currently.
Ah, that's ok then! No problem.
The major change in this PR is the extension of Sweeper to work with the new multichannel retina.
The smaller change is that Gaussian patterns in a composite now all have the same orientation, and are therefore swept in the same direction. This change is required to get meaningful direction maps with two direction patches of opposite direction preference within one orientation patch.