Skip to content

Add 2D conv #5

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

Merged
merged 5 commits into from
Jul 21, 2025
Merged

Add 2D conv #5

merged 5 commits into from
Jul 21, 2025

Conversation

Atticus1806
Copy link
Contributor

No description provided.

@Atticus1806 Atticus1806 self-assigned this Jul 8, 2025
Copy link
Member

@NeoLegends NeoLegends left a comment

Choose a reason for hiding this comment

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

The memristor forward is hard to review for correctness, so I trust you on the correctness because it is difficult to get it to run and have bugs at the same time. Seems like the bit-exponent off by one error is fixed for all modules?

So the rest is LGTM!

Requested changes because test is not run, see comment.

size = flat.shape[0]
positive_cells = CellArrayCPU(size)
negative_cells = CellArrayCPU(size)
for _ in range(num_cycles * 50):
Copy link
Member

Choose a reason for hiding this comment

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

Why * 50?

Copy link
Member

Choose a reason for hiding this comment

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

ah the other modules have it as well.. why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to have cycles independent we needed a value larger than the internal cycle history of synaptogen.
We found it easier to not expose this to the outside, otherwise you have to set cycles always with a factor, which we deemed not so nice.

@Atticus1806 Atticus1806 requested a review from NeoLegends July 15, 2025 10:37
Copy link
Member

@NeoLegends NeoLegends left a comment

Choose a reason for hiding this comment

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

🚢

@Atticus1806 Atticus1806 merged commit 1c45f43 into main Jul 21, 2025
5 checks passed
@Atticus1806 Atticus1806 deleted the bene_2d_conv branch July 21, 2025 07:28
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.

2 participants