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

Allow methods configs to be discovered from external packages. #1357

Merged
merged 4 commits into from
Feb 6, 2023

Conversation

jkulhanek
Copy link
Contributor

This PR implements a discovery mechanism for TrainerConfigs. The main objective is to allow other researchers to publish their own methods which use Nerfstudio without them needing to copy the source code. They will publish only the method implementation with TrainerConfig, and it will be automatically discovered and registered by Nerfstudio.

The implementation uses the package metadata to find the TrainerConfigs. In particular, it will look for the "nerfstudio.method_configs" entry point.

I believe this will improve the collaboration as contributors no longer need to clone the entire repository if they only want to try a small change. Also, this way, Nerfstudio's authors can wait if an implementation works well before porting it into the Nerfstudio codebase.

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.

Cool idea. How does this handle versioning? Ie does the external model define a nerfstudio version compatibility? Im just curious what happens if we make breaking changes to the nerfstudio repo (as it is still in pretty early development).

Also you can run ns-dev-test to fix the errors.

@jkulhanek
Copy link
Contributor Author

Thanks! The idea is for the external model to define the Nerfstudio version as python dependency. Therefore, breaking changes should not break compatibility as external packages will likely target a specific version of Nerfstudio.

Errors should be fixed. Not sure about the naming of the new class "MethodSpecification" which contains the TrainerConfig and the method description. What do you think?

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.

Name is good with me. Thanks for adding!

@tancik tancik merged commit 59f71ed into nerfstudio-project:main Feb 6, 2023
@machenmusik
Copy link
Contributor

Nice! Were you thinking that there is a built-in method that would be a good test case to show this decomposition in action, or was the idea to enable the functionality and the first method to use it would become the de facto example?

@jkulhanek
Copy link
Contributor Author

I thought of it in the context of adding new methods. I believe the Plenoxels could be a good candidate. Adding it directly to the Nerfstudio repo would make Nerfstudio harder to install while most people don’t need it.

yimingzhou1 added a commit to parallelsystems/nerfstudio that referenced this pull request Feb 7, 2023
* Fix normals visualization (nerfstudio-project#1341)

* Fix depth-supervised nerf when using z-buffer depth. (nerfstudio-project#1340)

* fix z buffer depth

* remove script

* Create shaders (nerfstudio-project#1344)

* New config parameter --viewer.codec defaults to VP8, but allows H264 (nerfstudio-project#1343)

* New config parameter --viewer.codec defaults to H264 rather than VP8

* make quantize clearer

* change codec to Literal

default is VP8 due to libx264 log spam, see nerfstudio-project#1343 (comment)

* isort

* pylint, including python 3.7 compatibility

* double-quote filenames in process-data commands so that on Windows, paths with spaces don't fail (nerfstudio-project#1347)

* double-quote filenames in commands so that on Windows, paths with spaces don't fail

* reformatting by black

* Improve viewer colormap choices (nerfstudio-project#1348)

* Consistent naming for depth maps in Polycam (nerfstudio-project#1352)

* Handle uncalibrated Metashape sensor (nerfstudio-project#1359)

This fix allows loading Metashape XML files which
contain an additional uncalibrated sensor.

Metashape XML format keeps unmatched images but
assignes a dummy sensor to them. The sensor has
no calibration data.

Co-authored-by: gilureta <gilureta@amazon.com>

* add viewer.local config property when viewing local instance (nerfstudio-project#1358)

If running local server instance, avoid using relays to communicate with the viewer.

* Allow methods configs to be discovered from external packages. (nerfstudio-project#1357)

* Allow method_config to be registered from external packages

* Add license

* Add MethodSpecification class + fix linter errors

---------

* Trace Stable Diffusion (nerfstudio-project#1362)

* Trace Stable Diffusion

* pylint

Co-authored by Terrance Wang

* Fix ns-train help (nerfstudio-project#1370)

* Improve ns-process-data images when using existing COLMAP model (nerfstudio-project#1371)

* Allow skipping copy+downscale of images in ns-process-data, fix bug when using --skip-colmap on images.bin with non-standard image names

* Rename flags and improve error messages

---------

Co-authored-by: Matthew Tancik <tancik@berkeley.edu>

---------

Co-authored-by: Matthew Tancik <tancik@berkeley.edu>
Co-authored-by: Jeffrey Hu <102542936+jeffreyhuparallel@users.noreply.github.com>
Co-authored-by: machenmusik <machenmusik@users.noreply.github.com>
Co-authored-by: ManuConcepBrito <33829944+ManuConcepBrito@users.noreply.github.com>
Co-authored-by: Francisca T. Gil Ureta <124404718+gilureta@users.noreply.github.com>
Co-authored-by: gilureta <gilureta@amazon.com>
Co-authored-by: Jonáš Kulhánek <jonas.kulhanek@live.com>
@vincent-lecrubier-skydio

I landed here after searching "Plenoxels" in this repo. If anyone works on integrating plenoxels with Nerfstudio I'd be interested!

@jkulhanek
Copy link
Contributor Author

I have it on my todo list. Unfortunately I didn’t have enough time lately. I will prepare something after the ICCV deadline (if nobody does it sooner).

chris838 pushed a commit to chris838/nerfstudio that referenced this pull request Apr 22, 2023
…tudio-project#1357)

* Allow method_config to be registered from external packages

* Add license

* Add MethodSpecification class + fix linter errors

---------
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.

4 participants