Skip to content

fix kernel_size np.int64 dtype in torchinfo - #538

Merged
bruAristimunha merged 1 commit into
braindecode:masterfrom
sliwy:fix_int_type_torchinfo
Sep 26, 2023
Merged

bruAristimunha merged 1 commit into
braindecode:masterfrom
sliwy:fix_int_type_torchinfo

Conversation

@sliwy

@sliwy sliwy commented Sep 13, 2023

Copy link
Copy Markdown
Collaborator

It solves #537 where torchinfo was throwing wrong kernel_size type.

@sliwy
sliwy force-pushed the fix_int_type_torchinfo branch from 6b3af23 to 5fddbc4 Compare September 13, 2023 19:31
@codecov

codecov Bot commented Sep 13, 2023 •

Copy link
Copy Markdown

Codecov Report

Merging #538 (5fddbc4) into master (ae843fe) will not change coverage.
The diff coverage is 100.00%.

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

@bruAristimunha

Copy link
Copy Markdown
Collaborator

Hey @sliwy,

I searched and look's like the Chambon is impact by this issue https://github.com/search?q=repo%3Abraindecode%2Fbraindecode%20.astype(int)&type=code

Can you make a small test to test all the models? You can use the models_dict proposed by @PierreGtch.

@PierreGtch

PierreGtch commented Sep 13, 2023 via email

Copy link
Copy Markdown
Collaborator

@sliwy

sliwy commented Sep 18, 2023

Copy link
Copy Markdown
Collaborator Author

Hey @bruAristimunha, I checked and for SleepStagerChambon2018 it works :) no clue why 😂

For the test, definitely, it could be something like this in the beginning but we need to know parameters to use or set good default values:

from braindecode.models.util import models_dict
@pytest.mark.parametrize("model_cls", models_dict.values())
def test_model_torchinfo(model_cls):
    print(model_cls(n_times=5000, n_chans=1, n_outputs=2))

@PierreGtch

Copy link
Copy Markdown
Collaborator

@sliwy not sure all the models can be initialized with their default parameters.

Plus I think we should test if the models accept inputs with the desired shape and return outputs with the correct shape:

from braindecode.models.util import models_dict
@pytest.mark.parametrize("model_cls", models_dict.values())
def test_model_torchinfo(model_cls):
    model = model_cls(n_times=5000, n_chans=1, n_outputs=2)
    assert model(torch.ones(3, 1, 5000)).shape == (3, 2)

@bruAristimunha

Copy link
Copy Markdown
Collaborator

Let's use two shapes:

For the BCI models, we will use BCI 2014 (1, 22, 1001),
For sleep, we will use Sleep Dataset (1, 2, 3000)./

@bruAristimunha

Copy link
Copy Markdown
Collaborator

As we have already opened other issues to address the requested extra, I will consider this issue closed and we will continue with another PR. Thank you very much @sliwy!!

@bruAristimunha
bruAristimunha merged commit 9cdde83 into braindecode:master Sep 26, 2023
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