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.

Sign up for GitHub

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

Jump to bottom

Ability to use vanilla pytorch instead of tcnn for Nerfacto #1936

Merged
merged 22 commits into from
May 25, 2023
Merged

Ability to use vanilla pytorch instead of tcnn for Nerfacto #1936

merged 22 commits into from
May 25, 2023

Conversation

ethanweber
Copy link
Collaborator

@ethanweber ethanweber commented May 16, 2023 •
edited

  • --pipeline.model.implementation tcnn is default but --pipeline.model.implementation torch can be used for vanilla pytorch. This will only work for the Nerfacto method at the moment.
  • Removed "nerfacto" from the blacklisted github actions tests.

The vanilla pytorch implementation is maybe ~4x slower. Looking into numerical equivalence is a good next step.

Example commands are as follows,

ns-train nerfacto --vis viewer+wandb --experiment-name aspen-tcnn --data data/nerfstudio/aspen --pipeline.model.implementation tcnn

and

ns-train nerfacto --vis viewer+wandb --experiment-name aspen-torch --data data/nerfstudio/aspen --pipeline.model.implementation torch

The quality looks very similar.

The main branch and this PR (which is "tcnn" here and still the default for nerfacto) should be equivalent. Here is a small sanity check.

image

@ethanweber ethanweber changed the title Ability to use vanilla pytorch instead of tcnn for Nerfacto [WIP] Ability to use vanilla pytorch instead of tcnn for Nerfacto May 16, 2023
tancik
tancik reviewed May 16, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@SauravMaheshkar SauravMaheshkar added enhancement New feature or request python Pull requests that update Python code labels May 17, 2023
@ethanweber ethanweber changed the title [WIP] Ability to use vanilla pytorch instead of tcnn for Nerfacto Ability to use vanilla pytorch instead of tcnn for Nerfacto May 18, 2023
@ethanweber ethanweber requested a review from tancik May 18, 2023 19:01
tancik
tancik reviewed May 23, 2023
View reviewed changes
nerfstudio/field_components/encodings.py Outdated Show resolved Hide resolved
nerfstudio/field_components/encodings.py Outdated Show resolved Hide resolved
nerfstudio/field_components/encodings.py Show resolved Hide resolved
nerfstudio/field_components/mlp.py Outdated Show resolved Hide resolved
nerfstudio/field_components/mlp.py Show resolved Hide resolved
nerfstudio/model_components/ray_samplers.py Show resolved Hide resolved
nerfstudio/scripts/train.py Outdated Show resolved Hide resolved
@tancik tancik linked an issue May 24, 2023 that may be closed by this pull request
Does anyone meet pretty low performance using TorchNerfactoField? #1927
Closed
tancik
tancik approved these changes May 25, 2023
View reviewed changes
Copy link
Contributor

@tancik tancik left a comment

Choose a reason for hiding this comment

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

LGTM

@ethanweber ethanweber merged commit bb6925b into main May 25, 2023
4 checks passed
@ethanweber ethanweber deleted the ethan/nerfacto-pytorch branch May 25, 2023 21:55
@tancik tancik mentioned this pull request May 25, 2023
Closed
@brentyi brentyi mentioned this pull request Jun 8, 2023
Closed
@ethanweber ethanweber mentioned this pull request Jun 12, 2023
Merged
@yurkovak yurkovak mentioned this pull request Aug 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tancik tancik tancik approved these changes

Assignees

@ethanweber ethanweber

Labels
enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Does anyone meet pretty low performance using TorchNerfactoField?
3 participants
@ethanweber @tancik @SauravMaheshkar