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

ns-render - render all images #2459

Merged
merged 6 commits into from
Oct 1, 2023
Merged

ns-render - render all images #2459

merged 6 commits into from
Oct 1, 2023

Conversation

jkulhanek
Copy link
Contributor

ns-render: add support for rendering of all images.

@jkulhanek
Copy link
Contributor Author

Now, I am not sure if this is needed. It seems like the ns-eval also supports exporting images?

@jkulhanek
Copy link
Contributor Author

ns-render dataset also supports exporting raw depth maps and has more control over generating the images. ns-eval seems to only export eval images in jpg...

Please let me know what you think...

@kerrj
Copy link
Collaborator

kerrj commented Sep 25, 2023

I'm not familiar with ns-eval, I think @FrederikWarburg made it. It does seem cleaner to have just one ns-render command which supports an 'eval' mode rather than an ns-eval script though. Fred do you have any thoughts?

@FrederikWarburg
Copy link
Contributor

Hi, I don't know about ns-eval. I agree that it does not seem super useful. I think what we did in nerfbuster is the easiest way to evaluate. Here we render out images using ns-render and save these renderings on disk (you can do this with either training or evaluation images. When you train your nerf you can decide how to split the train/eval set), and then we have a separate script that computes metrics. I believe ns-eval tries to do this without saving the renderings to disk.

@jkulhanek
Copy link
Contributor Author

Yes I agree. I was missing the functionality to render all train/eval images.

@f-dy
Copy link
Contributor

f-dy commented Sep 26, 2023

@jkulhanek another issue with ns-render is that it doesn't use the full set of intrinsics (unless it was fixed since #2059). Does this use all intrinsics, including the image size? Images may have different sizes.

With #2059, the "dataset" render mode is not necessary: I use ns-render interpolate --interpolation-steps 1 , and if you want to render all images (training and validation), you can add --train-split-fraction 1.0

Sorry I didn't have time to provide sample outputs for #2059 so it was never merged, but could you take a look at it?

@jkulhanek
Copy link
Contributor Author

@f-dy ns-render dataset should respect the intrinsics and image size. It generates images such that they can be compared with gt. What you are suggesting with ns-render interpolate solves a different problem, I believe. ns-render dataset renders different outputs individually (depth, img, segmasks). It allows users to generate raw depth predictions and raw image predictions (no compression). Furthermore, it uses the original file names which makes comparing with gt simple.
ns-render interpolate --interpolation-steps 1 suffers from the following issues:

  • You have to manually render train and test splits; the command doesn't allow you to render all splits
  • Val split is not supported at all
  • It generates single image merging outputs, you have to manually split it into individual images (depth, RGB)
  • It doesn't allow you to output raw depth maps or raw RGB values. These are required in some applications.
  • You have to find the original names of images, you only get their order as they were parsed by the data parser. This, again makes the comparison with gt more difficult as users would have to write scripts to dump the filenames from a parser.

Overall I think it's nice to have both command ns-render dataset and ns-render interpolate as the use cases are different. I think the functionality of ns-render dataset was missing for a long time and users (myself included) had to write their own scripts to do it.

@f-dy
Copy link
Contributor

f-dy commented Sep 27, 2023

I was using ns-render interpolate --interpolation-steps 1 as a workaround for the missing ns-render dataset (and I agree the filenames was an issue). If the latter solves everything then that's great! please merge asap!

@jkulhanek
Copy link
Contributor Author

Can we merge this?

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

@jkulhanek jkulhanek merged commit b78d0fa into main Oct 1, 2023
@jkulhanek jkulhanek deleted the jkulhanek/render-dataset branch October 1, 2023 20:12
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.

5 participants