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

Document speed comparison #2072

Merged
merged 17 commits into from
Dec 17, 2020
Merged

Document speed comparison #2072

merged 17 commits into from
Dec 17, 2020

Conversation

Borda
Copy link
Member

@Borda Borda commented Jun 4, 2020

What does this PR do?

Extend documentation on how the Pl is running almost the same fast as vanilla PT
Follow-up from #1587

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@Borda Borda added docs Documentation related ci Continuous Integration labels Jun 4, 2020
@mergify mergify bot requested a review from a team June 4, 2020 13:06
@Borda Borda mentioned this pull request Jun 4, 2020
@edenlightning
Copy link
Contributor

  • make sure to add a todo for memory parity to catch leaks

@Borda
Copy link
Member Author

Borda commented Jun 5, 2020

  • make sure to add a todo for memory parity to catch leaks

memory parity will be separate PR as it has to be developed first... 🐰

@codecov
Copy link

codecov bot commented Jun 5, 2020

Codecov Report

Merging #2072 (4a2b193) into master (1b599ff) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #2072   +/-   ##
======================================
  Coverage      93%     93%           
======================================
  Files         134     134           
  Lines        9928    9928           
======================================
  Hits         9242    9242           
  Misses        686     686           

@Borda
Copy link
Member Author

Borda commented Jun 5, 2020

config with k80 for MNIST dataset training on 15 epochs

# run 1
pt_times: [78.08037253795192, 77.65994176699314, 77.16897976701148, 75.97896099602804, 76.10812993405852, 77.09142099390738, 76.38175202498678, 76.61454085202422, 76.64365514100064, 76.46872986794915, 76.58440757205244, 75.86799949593842, 75.37725775991566, 77.95835449907463, 78.78175812901463, 78.05797199800145, 77.72009757102933, 77.71631882700603, 78.06795359996613]
pl_times: [82.00195560394786, 82.00709507102147, 81.805350583978, 81.26589621405583, 80.73785846401006, 80.55167946091387, 81.38005885004532, 81.20296528504696, 80.27230604400393, 80.75083769799676, 80.93695296195801, 80.93231990898494, 80.48659746092744, 81.36925745301414, 80.96931200707331, 80.6680577499792, 80.6818772739498, 81.12940620095469, 82.45112453098409]
# run 2
pt_times: [72.35451863205526, 71.80189288198017, 71.9814427359961, 71.92960607400164, 71.40658942295704, 72.75710438506212, 73.50131551991217, 73.19857055298053, 73.15937736898195, 72.86832409596536, 72.34491985896602, 72.94093307398725, 74.36359175201505, 74.67142662103288, 74.2357223139843, 74.14241052698344, 74.0228865040699, 74.53661659103818, 74.28851658396889] 
pl_times: [75.84128665702883, 76.46811646199785, 75.47480550594628, 75.47422935103532, 74.94576230400708, 75.44481814303435, 75.91588587092701, 75.67014791606925, 75.5910986890085, 75.1219390759943, 76.10618040000554, 75.65077104105148, 74.04137368802913, 74.90914194402285, 77.16411820601206, 77.57090287993196, 76.66609343804885, 76.90365857002325, 76.51128543494269]
# run 3
pt_times: [82.37473194301128, 82.97512700094376, 83.08479252899997, 83.83531925699208, 83.09167189500295, 82.96716181200463, 83.35179279604927, 83.84988487605006, 83.43091613892466, 82.8106702209916, 83.24989012500737, 83.35911275492981, 83.28093389503192, 82.61112239805516, 83.16935679502785, 82.62005160597619, 81.47776250902098, 82.40548102708999, 83.22479931509588]
pl_times: [85.93682769290172, 85.97661709308159, 87.47603658295702, 87.49147263797931, 87.64337389892898, 87.64634780201595, 87.6974927430274, 87.16452221293002, 86.25435132498387, 86.32870114094112, 85.65250021102838, 86.81863707804587, 85.7048433390446, 87.24022658006288, 86.82864296494517, 86.9083100440912, 86.09307709196582, 85.65272393194027, 85.86923094291706]

image

@edenlightning edenlightning added this to the 0.8.0 milestone Jun 8, 2020
@Borda Borda modified the milestones: 0.8.0, 0.9.0 Jun 9, 2020
@awaelchli awaelchli modified the milestones: 0.9.0, 0.9.x Aug 8, 2020
@edenlightning edenlightning modified the milestones: 0.9.x, 1.0 Oct 4, 2020
@Borda Borda added this to the 1.0.x milestone Oct 20, 2020
@stale
Copy link

stale bot commented Nov 3, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need further help see our docs: https://pytorch-lightning.readthedocs.io/en/latest/CONTRIBUTING.html#pull-request or ask the assistance of a core contributor here or on Slack. Thank you for your contributions.

@stale stale bot added the won't fix This will not be worked on label Nov 3, 2020
@ydcjeff ydcjeff removed the won't fix This will not be worked on label Nov 3, 2020
@edenlightning edenlightning modified the milestones: 1.0.x, 1.0.7 Nov 10, 2020
@Borda Borda modified the milestones: 1.0.7, 1.0.x Nov 11, 2020
@edenlightning edenlightning modified the milestones: 1.0.x, 1.0.7 Nov 13, 2020
@Borda
Copy link
Member Author

Borda commented Dec 16, 2020

@Borda Borda requested a review from SkafteNicki December 17, 2020 07:48
@Borda Borda added the ready PRs ready to be merged label Dec 17, 2020
Copy link
Contributor

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

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

I have the feeling this does not fit into the "best practices" section. But there are not many other options. Maybe @edenlightning has a good idea? :)

@awaelchli
Copy link
Contributor

awaelchli commented Dec 17, 2020

maybe @SeanNaren wants to include his ddp benchmarking results here too? :) I've seen some great charts somewhere

@Borda
Copy link
Member Author

Borda commented Dec 17, 2020

I have the feeling this does not fit into the "best practices" section. But there are not many other options. Maybe @edenlightning has a good idea? :)

I think we can move it anytime, I just need this as base for memory benchmark :]

maybe @SeanNaren wants to include his ddp benchmarking results here too? :) I've seen some great charts somewhere

that would be great, so another PR?

@Borda Borda merged commit b16441f into master Dec 17, 2020
@Borda Borda deleted the docs/speed branch December 17, 2020 11:04
awaelchli pushed a commit that referenced this pull request Dec 18, 2020
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Borda added a commit that referenced this pull request Dec 23, 2020
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Borda added a commit that referenced this pull request Dec 29, 2020
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Borda added a commit that referenced this pull request Jan 4, 2021
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Borda added a commit that referenced this pull request Jan 4, 2021
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Borda added a commit that referenced this pull request Jan 5, 2021
* docs

* script

* dump

* desc

* import

* import

* if

* norm

* t

* finished

* isort

* typing

Co-authored-by: Nicki Skafte <[email protected]>

* xlabel

* pandas

* time

Co-authored-by: Nicki Skafte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration docs Documentation related ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants