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

Restructuring C++ project #3146

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Dec 9, 2020

POC for restructuring the C++ project based on discussions with @fmassa

  • Moving operator related files in ops.
  • Moving image/video related files in io.
  • Move autocast in a separate folder in ops.
  • Hide autograd forward/backward methods in an anonymous namespace.
  • Reduce unnecessary header inclusions in models and io.

@datumbox datumbox force-pushed the refactoring/cpp_project_structure branch 8 times, most recently from 47c8bc9 to ac94f03 Compare December 9, 2020 23:32
@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #3146 (3420106) into master (4d3a309) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3146   +/-   ##
=======================================
  Coverage   72.75%   72.75%           
=======================================
  Files          99       99           
  Lines        8979     8979           
  Branches     1431     1431           
=======================================
  Hits         6533     6533           
  Misses       1999     1999           
  Partials      447      447           

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 4d3a309...3420106. Read the comment docs.

@datumbox datumbox force-pushed the refactoring/cpp_project_structure branch 3 times, most recently from 6ee60e2 to df7f550 Compare December 10, 2020 01:47
@datumbox datumbox changed the title [WIP] Restructuring C++ project Restructuring C++ project Dec 10, 2020
@datumbox datumbox requested a review from fmassa December 10, 2020 02:07
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

PR looks great, thanks a lot!

A few high level things we could do in a follow-up:

  • split (if possible) the autograd bits into its own folder, as it's a backend as the others
  • re-structure the video parts, as now they are scattered over 3 different folders (but needs some planning before doing it)

Summary:
* Reduce unnecessary header inclusions in models and io.

* Move autocast to separate folder and hide autograd implementation in an anonymous namespace.

* Moving files in subfolders.

Reviewed By: fmassa

Differential Revision: D25461523

fbshipit-source-id: 756eeb6848aacaa474de4825ed4c1045d17e2cea
@datumbox datumbox force-pushed the refactoring/cpp_project_structure branch from df7f550 to 3420106 Compare December 10, 2020 18:34
@datumbox datumbox merged commit 7d831a2 into pytorch:master Dec 10, 2020
@datumbox
Copy link
Contributor Author

@fmassa Sounds good.

The autograd move is now completed at PR #3154. For the cleanup and restructuring I raised an issue at #3155.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants