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

Fourier Feature encodings and polyhedron encodings #2463

Merged
merged 18 commits into from
Oct 2, 2023

Conversation

Mxbonn
Copy link
Contributor

@Mxbonn Mxbonn commented Sep 24, 2023

Rework RFFEncoding to be a subclass of the more general Fourier Feature Encodings and introduce Polyhedron encodings as introduced in mipnerf360.

Rework RFFEncoding to be a subclass of the more general Fourier Feature
Encodings and introduce Polyhedron encodings as introduced in
mipnerf360.
@Mxbonn
Copy link
Contributor Author

Mxbonn commented Sep 24, 2023

pyright is really not liking the self.b_matrix as it comes from a buffer. Anyone knows how to tell it that self.b_matrix.shape is a tuple of ints? For now just using ignore.
Slightly unrelated but I cannot get to run pyright locally as it does not recognize jaxtyping.

nerfstudio/utils/math.py Outdated Show resolved Hide resolved
@jkulhanek
Copy link
Contributor

Thank you for this PR!

@jkulhanek
Copy link
Contributor

pyright is really not liking the self.b_matrix as it comes from a buffer. Anyone knows how to tell it that self.b_matrix.shape is a tuple of ints? For now just using ignore. Slightly unrelated but I cannot get to run pyright locally as it does not recognize jaxtyping.

What is the exact error?

@Mxbonn
Copy link
Contributor Author

Mxbonn commented Sep 24, 2023

pyright is really not liking the self.b_matrix as it comes from a buffer. Anyone knows how to tell it that self.b_matrix.shape is a tuple of ints? For now just using ignore. Slightly unrelated but I cannot get to run pyright locally as it does not recognize jaxtyping.

What is the exact error?

error: "__getitem__" method not defined on type "Module" (reportGeneralTypeIssues) for the line that does: out_dim = self.b_matrix.shape[1] * self.num_frequencies * 2

@jkulhanek
Copy link
Contributor

Can you just add an assert at the beginning of the block code where you have the error?

assert isinstance(self.b_matrix, torch.Tensor)

@jkulhanek
Copy link
Contributor

jkulhanek commented Sep 24, 2023

Slightly unrelated but I cannot get to run pyright locally as it does not recognize jaxtyping.

I think I had to uninstall torchtyping, but I don't remember, sorry :/

Copy link
Contributor

@jkulhanek jkulhanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove type:ignore

@Mxbonn
Copy link
Contributor Author

Mxbonn commented Sep 24, 2023

Can you just add an assert at the beginning of the block code where you have the error?

assert isinstance(self.b_matrix, torch.Tensor)

Thanks, that indeed did the trick! I have dropped the ignore lines now.


Adapted from https://github.com/google-research/multinerf/blob/5b4d4f64608ec8077222c52fdf814d40acc10bc1/internal/geopoly.py
Args:
base_shape: string, the name of the starting polyhedron, must be either
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This have to be offset. See the docs error:

/home/runner/work/nerfstudio/nerfstudio/nerfstudio/utils/math.py:docstring of nerfstudio.utils.math.generate_polyhedron_basis:8: ERROR: Unexpected indentation.
/home/runner/work/nerfstudio/nerfstudio/nerfstudio/utils/math.py:docstring of nerfstudio.utils.math.generate_polyhedron_basis:9: WARNING: Block quote ends without a blank line; unexpected unindent.
/home/runner/work/nerfstudio/nerfstudio/nerfstudio/utils/math.py:docstring of nerfstudio.utils.math.generate_polyhedron_basis:14: WARNING: Definition list ends without a blank line; unexpected unindent.

eps: float, a small number used to determine symmetries.

Returns:
basis: a matrix with shape [3, n].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

return weights


def tesselate_geodesic(
Copy link
Contributor

@jkulhanek jkulhanek Sep 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we perhaps make private functions (not needed to be exported) to start with underscore (_)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, and is it fine that I put them in utils/math.py or would you prefer to have them in a new separate utils file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its fine to have them there I think

@jkulhanek
Copy link
Contributor

@tancik , do you agree with merging?

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, nice abstraction

@jkulhanek jkulhanek enabled auto-merge (squash) October 2, 2023 20:51
@jkulhanek jkulhanek merged commit 0d746b1 into nerfstudio-project:main Oct 2, 2023
4 checks passed
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