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

Make dataset.ishuffle optional #1529

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Conversation

krajsek
Copy link
Member

@krajsek krajsek commented Jun 16, 2024

Due Diligence

  • General:
    • title of the PR is suitable to appear in the release notes
  • Implementation:
    • unit tests: no unit test added, so far datatools.py has no unit tests
    • unit tests: no, same as above
    • documentation updated where needed

Description

So far, the Heat dataset requires the implementation of an ishuffle method that defines the shuffling of data between epochs. This is not compatible with the PyTorch dataset class. To enable the use of PyTorch dataset classes, ishuffle is now optional.

Type of change

  • New feature

Does this change modify the behaviour of other functions? If so, which?

no

@krajsek krajsek added the Data-parallel NNs / DASO everythin related to data-parallel neural networks and/or the DASO optimizer label Jun 16, 2024
Copy link
Contributor

Thank you for the PR!

Copy link

codecov bot commented Jun 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.80%. Comparing base (586788e) to head (c27e822).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1529   +/-   ##
=======================================
  Coverage   91.80%   91.80%           
=======================================
  Files          80       80           
  Lines       11772    11773    +1     
=======================================
+ Hits        10807    10808    +1     
  Misses        965      965           
Flag Coverage Δ
unit 91.80% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@ClaudiaComito ClaudiaComito added this to the 1.5.0 milestone Jun 17, 2024
@ClaudiaComito ClaudiaComito changed the title added optional dataset.ishuffle Make dataset.ishuffle optional Jun 19, 2024
Copy link
Contributor

@ClaudiaComito ClaudiaComito left a comment

Choose a reason for hiding this comment

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

Great, thanks @krajsek !

Copy link
Contributor

Thank you for the PR!

@ClaudiaComito ClaudiaComito merged commit c41bbac into main Jun 20, 2024
53 checks passed
@ClaudiaComito ClaudiaComito deleted the features/1511-optional_shuffling branch June 20, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data-parallel NNs / DASO everythin related to data-parallel neural networks and/or the DASO optimizer interoperability merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants