Skip to content

changed default value of final_conv_length to 'auto' in Deep4Net #535

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 3 commits into from
Sep 19, 2023

Conversation

sliwy
Copy link
Collaborator

@sliwy sliwy commented Sep 12, 2023

After last changes, default value of final_conv_length in Deep4Net is None and code fails. Before it was mandatory to provide this paramter. I'm setting default to `"auto" -> it should not break earlier code as before it had to be specified.

Copy link
Collaborator

@bruAristimunha bruAristimunha left a comment

Choose a reason for hiding this comment

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

Lgtm

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #535 (1c4376a) into master (ae843fe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #535   +/-   ##
=======================================
  Coverage   84.48%   84.48%           
=======================================
  Files          63       63           
  Lines        4653     4653           
=======================================
  Hits         3931     3931           
  Misses        722      722           

@sliwy sliwy force-pushed the fix_deep4net_default branch from 80bec63 to 398dafd Compare September 12, 2023 20:57
@robintibor
Copy link
Contributor

good catch! and what about shallow? should be same Problem there no?

@sliwy
Copy link
Collaborator Author

sliwy commented Sep 18, 2023

for shallow filter_time_length is set to 25 by default, it was the same before recent changes and it still works as before :)

@bruAristimunha bruAristimunha enabled auto-merge (squash) September 19, 2023 15:01
@bruAristimunha bruAristimunha merged commit cb5cad0 into braindecode:master Sep 19, 2023
@bruAristimunha
Copy link
Collaborator

Thank you @sliwy =)

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