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

adaptive regularization added #243

Merged
merged 8 commits into from
Jul 10, 2024
Merged

adaptive regularization added #243

merged 8 commits into from
Jul 10, 2024

Conversation

pollytur
Copy link
Collaborator

The code to add adaptive regularisation for full gaussian readouts

@pollytur pollytur requested a review from MaxFBurg June 17, 2024 13:05
Copy link
Member

@MaxFBurg MaxFBurg left a comment

Choose a reason for hiding this comment

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

Very nice work @pollytur ! I made a few very small suggestions, mainly on naming consistency and structure consistency, most of them you could commit right within the PR (I used the suggest function). Sorry again this PR slipped through on my todo list and you needed to wait so long.

neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
@MaxFBurg MaxFBurg self-requested a review July 9, 2024 14:12
MaxFBurg
MaxFBurg previously approved these changes Jul 10, 2024
Copy link
Member

@MaxFBurg MaxFBurg left a comment

Choose a reason for hiding this comment

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

I think there is a typo in one of the error strings, I made a suggestion, otherwise looks good!

neuralpredictors/layers/readouts/gaussian.py Outdated Show resolved Hide resolved
@pollytur
Copy link
Collaborator Author

@MaxFBurg I think you would need to reapprove after I committed the suggested change

@MaxFBurg MaxFBurg merged commit 5e86a82 into sinzlab:main Jul 10, 2024
5 checks passed
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