tiny-cuda-nn icon indicating copy to clipboard operation
tiny-cuda-nn copied to clipboard

Features per Level vs Features per Entry

Open jc211 opened this issue 2 years ago • 2 comments

Hi, is the below not supposed to be N_FEATURES_PER_ENTRY rather than N_FEATURES_PER_LEVEL. It's just a naming thing but its kind of confusing. I think the paper refers to this as F and there its called feature per entry.

https://github.com/NVlabs/tiny-cuda-nn/blob/bb2b1f8a5cf4632778edff707e5e18a5247d5c93/include/tiny-cuda-nn/encodings/grid.h#L524

jc211 avatar Feb 28 '22 05:02 jc211

I think I may understand now what is meant. I think N_FEATURES_PER_LEVEL means more like N_FEATURES_PER_ENTRY_PER_LEVEL and n_features is then N_FEATURES_PER_ENTRY once all levels are concatenated. Still a bit confusing to me because when reading it, it sounds like the whole level only has 2 features and I have to keep reminding myself that is not what is meant. I don't have any clear suggestions but perhaps DIM_ for dimension is more appropriate than N_.

If this is too much of a nitpick, please close the issue. In any event, I'm enjoying going through the code. Thanks for publishing it.

jc211 avatar Feb 28 '22 08:02 jc211

I think you're right about N_FEATURES_PER_ENTRY being the better choice in any case. It's clearer in its meaning + we're already using this terminology in the instant-ngp paper.

The idea of naming it n_features_per_level was that the output dimensionality of the encoding ends up as: n_features_per_level * n_levels.

Tom94 avatar Feb 28 '22 08:02 Tom94