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

Update nerfstudio_dataparser.py #2801

Closed
wants to merge 1 commit into from

Conversation

ichsan2895
Copy link

@ichsan2895 ichsan2895 commented Jan 20, 2024

This is initial code:

I fix to --load-3D-points set to True by default.

Why I set True by default? Because I see another recommendation in this line for set it true

Meanwhile, we already switching from colmap dataparser to nerfstudio dataparser #2791

load-3D-points set to True by default
@ichsan2895
Copy link
Author

If we don't declare colmap --load-3D-points, it was False by Default
ns-train nerfacto --data ./Nerfstudio/COLMAP_PLY --output-dir ./Nerfstudio/outputs_v1 --pipeline.model.eval-num-rays-per-chunk 8192 --pipeline.datamanager.train-num-rays-per-batch 1024 --pipeline.datamanager.eval-num-rays-per-batch 1024

image

If I declare it
ns-train nerfacto --data ./Nerfstudio/COLMAP_PLY --output-dir ./Nerfstudio/outputs_v1 --pipeline.model.eval-num-rays-per-chunk 8192 --pipeline.datamanager.train-num-rays-per-batch 1024 --pipeline.datamanager.eval-num-rays-per-batch 1024 colmap --load-3D-points True
image

@brentyi
Copy link
Collaborator

brentyi commented Jan 20, 2024

Hm yeah, it seems reasonable to make these two consistent. But this change only impacts non-splatting methods where the points aren't used, right? (like nerfacto)

For the nerfstudio dataparser, this is currently overridden to True by default for splatting:

datamanager=FullImageDatamanagerConfig(
dataparser=NerfstudioDataParserConfig(load_3D_points=True),
),

I also verified this in the helptext from ns-train splatfacto nerfstudio-data --help.

@ichsan2895
Copy link
Author

ichsan2895 commented Jan 20, 2024

Hm yeah, it seems reasonable to make these two consistent. But this change only impacts non-splatting methods where the points aren't used, right? (like nerfacto)

For the nerfstudio dataparser, this is currently overridden to True by default for splatting:

datamanager=FullImageDatamanagerConfig(
dataparser=NerfstudioDataParserConfig(load_3D_points=True),
),

I also verified this in the helptext from ns-train splatfacto nerfstudio-data --help.

yeah, you are alright. The --load-3D-points is true when I use ns-train splatfacto by default. Thats why I make the other method as same as splatfacto for consistency.

@brentyi
Copy link
Collaborator

brentyi commented Jan 21, 2024

Are there any other methods that will use the points?

The one argument I see for keeping the discrepancy is that the colmap parser always has points available, while the nerfstudio parser only sometimes has points.

If we always set load_3D_points=True for the nerfstudio parser, there will be many cases where this warning will be hit, even for models like nerfacto:

if not self.prompted_user:
CONSOLE.print(
"[bold yellow]Warning: load_3D_points set to true but no point cloud found. splatfacto will use random point cloud initialization."
)

This might be confusing for people.

@kerrj
Copy link
Collaborator

kerrj commented Jan 29, 2024

should this be closed?

@ichsan2895
Copy link
Author

Okay, let close it since you said this:

If we always set load_3D_points=True for the nerfstudio parser, there will be many cases where this warning will be hit, even for models like nerfacto

@ichsan2895 ichsan2895 closed this Feb 4, 2024
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