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

Refactor #15

Merged
merged 34 commits into from
May 2, 2023
Merged

Refactor #15

merged 34 commits into from
May 2, 2023

Conversation

atenagm1375
Copy link
Collaborator

Major changes:

  • The user is not required to define the whole config dictionary. They only need to call make or update_make on default config classes to generate all and/or update some config keys, respectively.
  • The input and output layers are now fully defined outside of the CorticalColumn. InputLayer contains two separate NeuronGroups for sensory and location populations. Likewise, the OutputLayer contains the motor and representation NeuronGroups.
  • The user now has control on defining either of sensory or location (or both) NeuronGroups.

Copy link
Member

@realamirhe realamirhe left a comment

Choose a reason for hiding this comment

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

LGTM

for batch in self.dataloader:
batch_x = batch[0] if self.have_sensory else None
batch_loc = batch[self.have_sensory] if self.have_location else None
batch_y = batch[-1] if self.have_label else None
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this better, might add other things to dataloader in near future

Suggested change
batch_y = batch[-1] if self.have_label else None
batch_y = batch[1] if self.have_label else None

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, The label should always be at the end. data loader can pass extra items before the label and after sensory and location data.


if batch_y is not None:
batch_y = batch_y.to(self.device)
each_instance = num_instance // torch.numel(batch_y)
Copy link
Member

Choose a reason for hiding this comment

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

It is not clear what variable num_instance refers to

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's the number of iterations a batch will last.

if LAYER_TIMESTAMPS["InputDataset"] not in behavior:
behavior[LAYER_TIMESTAMPS["InputDataset"]] = SpikeNdDataset(
dataloader=input_dataloader,
N_sensory=sensory_data_dim,
Copy link
Member

Choose a reason for hiding this comment

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

rename these variable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Some have changed in the new commit. any recommendation for 'N_sensory' and 'N_location' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about sensory_size and location_size?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not the size of the population. It's N as N-Dimension. e.g. 2d image for the sensory population...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Then if size makes confusion, we can use ndim


def __get_ng(self, net, size, tag, trace, setter, user_defined=None):
behavior = {
NEURON_TIMESTAMPS["Fire"]: setter(),
Copy link
Member

Choose a reason for hiding this comment

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

could we add a simple comment as a doc for future doc reference

from conex.nn.Structure.Layers import Layer
from conex.nn.Modules.topological_connections import StructuredSynapseGroup

from pymonntorch import SynapseGroup, NeuronGroup, TaggableObject

# TODO: handle multi-scale
# TODO: str.removesuffix needs python 3.9
Copy link
Member

Choose a reason for hiding this comment

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

update the project base config and force devs to use python 3.9+

Amir.H Ebrahimi and others added 3 commits May 1, 2023 15:23
* TimeStep -> TimeResolution
* timesteps -> time_window
* timestamps -> priorities
* Reward -> Payoff
@saeedark saeedark merged commit a3e0dd5 into main May 2, 2023
@atenagm1375 atenagm1375 deleted the refactor branch May 3, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment