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

Added shuffling to trainset dataloaders, no shuffle by default for ot… #234

Merged
merged 1 commit into from
Aug 17, 2023
Merged

Conversation

cfmelend
Copy link
Contributor

By default casanovo does not shuffle the dataloaders' batches. This quick patch simply adds an arg in dataloaders.py, within the DeNovoDataModule class, and within the _make_loader() method that determines shuffling. This param is set to false by default and set to true only when _make_loader() is called within the train_dataloader() method.

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #234 (bbce330) into dev (514db80) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #234      +/-   ##
==========================================
+ Coverage   89.13%   89.24%   +0.11%     
==========================================
  Files          12       12              
  Lines         902      902              
==========================================
+ Hits          804      805       +1     
+ Misses         98       97       -1     
Files Changed Coverage Δ
casanovo/denovo/dataloaders.py 98.18% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cfmelend cfmelend added the bug Something isn't working label Aug 17, 2023
Copy link
Collaborator

@wfondrie wfondrie 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 to me!

@cfmelend cfmelend merged commit 51221f9 into Noble-Lab:dev Aug 17, 2023
6 checks passed
@cfmelend cfmelend deleted the shuffle branch August 17, 2023 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants