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

Removes obsolete independent_bn_bias argument #221

Merged
merged 9 commits into from
Mar 8, 2024

Conversation

MaxFBurg
Copy link
Member

@MaxFBurg MaxFBurg commented Mar 7, 2024

Addressing #219

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 7.52%. Comparing base (5994426) to head (e354212).

Files Patch % Lines
neuralpredictors/layers/cores/conv3d.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #221      +/-   ##
========================================
+ Coverage   7.50%   7.52%   +0.02%     
========================================
  Files         58      58              
  Lines       5982    5965      -17     
  Branches    1018    1012       -6     
========================================
  Hits         449     449              
+ Misses      5496    5479      -17     
  Partials      37      37              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pollytur
Copy link
Collaborator

pollytur commented Mar 7, 2024

I would keep it on pause while the examples in the notebooks folder are not updated because currently there is nnsysident there and it actually uses this argument

@pollytur
Copy link
Collaborator

pollytur commented Mar 7, 2024

After discussing we decided to remove the example later and in the repositories, who depend on this parameter, they might specify the commit to inherit. There is the independent_bn_bias in the 3d core, but this should be refactored anyway

@pollytur pollytur self-requested a review March 7, 2024 14:49
pollytur
pollytur previously approved these changes Mar 7, 2024
@MaxFBurg
Copy link
Member Author

MaxFBurg commented Mar 7, 2024

Merge #221 226 before merging this PR to avoid merge conflicts

@MaxFBurg MaxFBurg requested a review from pollytur March 8, 2024 10:52
@pollytur
Copy link
Collaborator

pollytur commented Mar 8, 2024

@MaxFBurg I guess its now black and should be fine after
Thanks for solving the merge conflict!

@pollytur pollytur mentioned this pull request Mar 8, 2024
@pollytur pollytur merged commit 825e73a into sinzlab:main Mar 8, 2024
7 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