-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Quickstart Guide Fixes #531
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! I hope it wasn't too hard to get up and running.
Added one minor comment.
scripts/run_eval.py
Outdated
@@ -151,7 +151,7 @@ class RenderTrajectory: | |||
# Path to config YAML file. | |||
load_config: Path | |||
# Name of the renderer output to use. rgb, depth, etc. | |||
rendered_output_name: str = "rgb" | |||
rendered_output_name: str = "rgb_fine" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep this default to rgb
, the method we expect people to use, intant-ngp
, doesn't return a coarse and fine.
This is a bit confusing for the user. Perhaps we should should add some logic like
if rendered_output_name not in outputs:
console.rule("Error", style="red")
console.print(f"Could not find {rendered_output_name} in the model outputs", justify="center")
console.print(f"Please set --rendered_output_name to one of: {outputs.keys()}")
sys.exit(1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for future branches, can you append your name to the beginning for organizational reasons (ie david/quickstart-changes). No need to this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcallisterdavid just bumping, not sure if you saw Matt's comments here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Ethan, I got it set up on the new machine and am working through some issues. I should have some changes out tonight!
* key fixes * Update README.md * Update run_eval.py * Better error reporting * lint * Removed rgb fine change * Readme updates * Justify center
No description provided.