Skip to content

ns-render - render all images - #2459

Merged
jkulhanek merged 6 commits into
mainfrom
jkulhanek/render-dataset
Oct 1, 2023
Merged

ns-render - render all images#2459
jkulhanek merged 6 commits into
mainfrom
jkulhanek/render-dataset

Conversation

@jkulhanek

Copy link
Copy Markdown
Contributor

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

@jkulhanek

Copy link
Copy Markdown
Contributor Author

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

@jkulhanek

Copy link
Copy Markdown
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

kerrj commented Sep 25, 2023

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@f-dy

f-dy commented Sep 26, 2023

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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

f-dy commented Sep 27, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

Can we merge this?

@tancik tancik left a comment

Copy link
Copy Markdown
Contributor

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