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

Glm class restructure #41

Merged
merged 261 commits into from
Dec 14, 2023
Merged

Glm class restructure #41

merged 261 commits into from
Dec 14, 2023

Conversation

BalzaniEdoardo
Copy link
Collaborator

@BalzaniEdoardo BalzaniEdoardo commented Aug 29, 2023

PR Overview:

In this PR, I aim to:

  1. Establish architecture principles for models within the neurostatslib package.
  2. Define a NoiseModel abstract class and a concrete PoissonNoiseModel object to implement the log-likelihood and emission probability.
  3. Define an abstract Solver class and various concrete subclasses to set up the optimization problems for different objective functions.
  4. Provide concrete implementations of the GLM and RecurrentGLM classes that rely on solver and noise model objects.

Review Suggestions:

I recommend starting the review by reading the developer notes 02 to 05 before delving into the code. These notes provide the rationale and a description of how I structured the classes and why.

Thank you in advance for reviewing this.
Cheers,
Edoardo

@BalzaniEdoardo BalzaniEdoardo marked this pull request as draft August 29, 2023 16:06
Copy link
Member

@billbrod billbrod left a comment

Choose a reason for hiding this comment

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

Looks good! Only the small changes we've already talked about.

Copy link
Member

@billbrod billbrod left a comment

Choose a reason for hiding this comment

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

Some small typo fixes, then this is ready to go! I think the developer notes and docs will need some more work, but that's a job for a separate PR.

src/nemos/simulation.py Outdated Show resolved Hide resolved
src/nemos/simulation.py Outdated Show resolved Hide resolved
src/nemos/simulation.py Outdated Show resolved Hide resolved
BalzaniEdoardo and others added 4 commits December 14, 2023 12:16
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
@BalzaniEdoardo BalzaniEdoardo merged commit da7c8c7 into main Dec 14, 2023
2 checks passed
@BalzaniEdoardo BalzaniEdoardo deleted the glm_class_restructure branch December 14, 2023 17: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.

3 participants