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.

Sign up for GitHub

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

Jump to bottom

Updating regression requirements #81

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Updating regression requirements #81

merged 6 commits into from
Jun 20, 2024

Conversation

mgeplf
Copy link
Collaborator

@mgeplf mgeplf commented May 15, 2024

  • Fitting the regression line for cerebellum group and SST cell type was using only 1 datapoint, and producing erroneous results. Now, the requirement for fitting the regression coefficient is have a minimum of 6 datapoints.
  • Added some debug statement for logging, which helps in checking how well the regression ran.

rai-pranav and others added 6 commits May 15, 2024 14:00
@rai-pranav
Updating regression requirements
d074b92 
* Fitting the regression line for cerebellum group and SST cell type was using only 1 datapoint, and producing erroneous results. Now, the requirement for fitting the regression coefficient is have a minimum of 6 datapoints.
* Added some debug statement for logging, which helps in checking how well the regression ran.
* Changed how compute_region_volumes function in atlas_densities.densities.utils is calculating the "id_volume" column. Earlier it was generating nan values which leads to errors downstream in the pipeline.
@mgeplf
make min_data_points a command like parameter for fit-average-densi…
74798d5 
…ties
@mgeplf
fix lints
16bdf61
@mgeplf
Merge remote-tracking branch 'origin/main' into fix_nan_id_counts
45f6c75
@mgeplf
update changelog
82730dc
@mgeplf
Merge remote-tracking branch 'origin/main' into fix_nan_id_counts
2c7f6a9
@mgeplf mgeplf requested a review from rai-pranav June 20, 2024 09:18
@mgeplf
Copy link
Collaborator Author

mgeplf commented Jun 20, 2024

@rai-pranav are you happy with the changes? If so, I guess you'll be the one who gives the scientific review, since it's originally your code :)

rai-pranav
rai-pranav approved these changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@rai-pranav rai-pranav left a comment

Choose a reason for hiding this comment

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

I am fine with the changes. I think with the default parameter min-data-points set to 1, the pipeline should behave similar to earlier case. In general, this number should be higher, and for a scientist to decide.

@mgeplf mgeplf merged commit a99649a into main Jun 20, 2024
5 checks passed
@mgeplf mgeplf deleted the fix_nan_id_counts branch June 20, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rai-pranav rai-pranav rai-pranav approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mgeplf @rai-pranav