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

WIP: Support generator-like data loader as PyTorch/libtorch #281

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

shendiaomo
Copy link
Collaborator

  1. This PR proposes a more natural syntax to write train loop: use range for like PyTorch and the C++ frontend
    for batch := range data.Loader(mnist) {
        ...
    }
  2. It's probably faster than the original version (about 18000:13000 on my MacBook Pro, to be confirmed)
  3. Some pitfalls remain and need more investigation.

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #281 into develop will decrease coverage by 0.16%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #281      +/-   ##
===========================================
- Coverage    79.95%   79.79%   -0.17%     
===========================================
  Files           37       37              
  Lines         1751     1737      -14     
===========================================
- Hits          1400     1386      -14     
  Misses         270      270              
  Partials        81       81              
Impacted Files Coverage Δ
vision/datasets/mnist.go 89.47% <100.00%> (-2.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 556963c...273b659. Read the comment docs.

torch.GC()
e.hasGCed = true
}
torch.SetTensorFinalizer(e.data.T)
Copy link
Owner

Choose a reason for hiding this comment

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

By calling torch.SetTensorFinalizer in Example.Data/Target, could we fix the bug #273 ? @shendiaomo @Yancey1989

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems this PR works with the C++ wrappered dataset, #273 using ImageLoader which is Go implementation, it seems this PR can not fix it.

// for batch := range Loader(myDataset) {
// ...
// }
func Loader(dataset Dataset) chan Example {
Copy link
Owner

Choose a reason for hiding this comment

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

This is a correct direction -- to use Go channel.

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