Skip to content

bars dataset + priors#30

Merged
cshewmake2 merged 10 commits intoalvin/olshausen_datasetfrom
alvin/data_reorg
Jun 1, 2022
Merged

bars dataset + priors#30
cshewmake2 merged 10 commits intoalvin/olshausen_datasetfrom
alvin/data_reorg

Conversation

@alvinzz
Copy link
Collaborator

@alvinzz alvinzz commented May 23, 2022

Refactor BarsDataset.

Adds Priors:

  • Spike-and-Slab
  • L0-norm

Updates unittests.

@alvinzz alvinzz linked an issue May 23, 2022 that may be closed by this pull request
@alvinzz alvinzz force-pushed the alvin/data_reorg branch 2 times, most recently from 8628020 to aee7cdb Compare May 24, 2022 18:48
@alvinzz alvinzz marked this pull request as ready for review May 24, 2022 18:49
@alvinzz alvinzz force-pushed the alvin/data_reorg branch from aee7cdb to a4ccf4e Compare May 24, 2022 18:51
@alvinzz alvinzz force-pushed the alvin/data_reorg branch from a4ccf4e to 718e687 Compare May 24, 2022 18:57
@belsten
Copy link
Collaborator

belsten commented May 24, 2022

General comment about formatting, we currently have a formatting style guide that is in another branch and will be merged. https://github.com/rctn/sparsecoding/blob/formatting/docs/contributing.md
Much of the stable code we have right now follows this guide.

@alvinzz alvinzz changed the base branch from alvin/reorg to formatting May 24, 2022 19:28
@alvinzz alvinzz force-pushed the alvin/data_reorg branch 2 times, most recently from 74b1796 to 008fb07 Compare May 25, 2022 00:34
@alvinzz alvinzz changed the title dataset + transform refactoring consolidate datasets May 25, 2022
@alvinzz alvinzz requested a review from ansaschmulbach May 25, 2022 00:38
@alvinzz alvinzz force-pushed the alvin/data_reorg branch 2 times, most recently from 72ab6b3 to 86c6206 Compare May 25, 2022 01:04
@cshewmake2
Copy link
Contributor

Thanks for all of the commits! For future reference: let's make sure each branch is adding its own feature. It's a lot easier to review and accept PRs quickly when they're atomic and add a specific new functionality or change.

@alvinz-matician
Copy link

Yeah old (bad) habits die hard. If it's too much I'm happy to break it up into smaller PRs. But it would be difficult to split everything from moving the BarsDataset to the last tests change, since the tests depend on that dataset.

@@ -0,0 +1,66 @@
from abc import ABC, abstractmethod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool. What's the difference between having Prior inherit abc.ABC vs just inheriting the default object type implicitly?
e.g.
class Prior(): or class Prior:

Does it just enforce the interface/inheritance structure somehow?

Copy link
Collaborator Author

@alvinzz alvinzz May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really enforce anything (being Python and all), but semantically it's designed for this purpose:

https://peps.python.org/pep-3119/

ABCs are simply Python classes that are added into an object’s inheritance tree to signal certain features of that object to an external inspector. Tests are done using isinstance(), and the presence of a particular ABC means that the test has passed.

In addition, the ABCs define a minimal set of methods that establish the characteristic behavior of the type. Code that discriminates objects based on their ABC type can trust that those methods will always be present. Each of these methods are accompanied by an generalized abstract semantic definition that is described in the documentation for the ABC. These standard semantic definitions are not enforced, but are strongly recommended.

@cshewmake2
Copy link
Contributor

I have yet to test out everything on my local machine. Will try to do that tomorrow sometime.

@alvinzz alvinzz force-pushed the alvin/data_reorg branch from c4081bb to 6e56a11 Compare May 25, 2022 16:06
@alvinzz alvinzz changed the base branch from formatting to alvin/olshausen_dataset May 25, 2022 16:10
@alvinzz alvinzz force-pushed the alvin/data_reorg branch from 6e56a11 to 0b9637f Compare May 25, 2022 16:12
@alvinzz alvinzz force-pushed the alvin/olshausen_dataset branch from fa88137 to 71d609d Compare May 25, 2022 16:14
@alvinzz alvinzz force-pushed the alvin/data_reorg branch from 0b9637f to 226334d Compare May 25, 2022 16:17
@alvinzz alvinzz changed the title consolidate datasets bars dataset + priors May 25, 2022
@alvinzz
Copy link
Collaborator Author

alvinzz commented May 25, 2022

Updated to have Numpy-style docstrings & pruned some commits into other PRs.

Copy link
Collaborator

@ligeralde ligeralde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make variable names all explicit (e.g., D --> dimension)

@cshewmake2 cshewmake2 merged commit 6e86695 into alvin/olshausen_dataset Jun 1, 2022
@cshewmake2 cshewmake2 deleted the alvin/data_reorg branch June 1, 2022 22:18
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 this pull request may close these issues.

implement torchvision data transform

5 participants