nerfstudio-project / nerfacc

A General NeRF Acceleration Toolbox in PyTorch.
https://www.nerfacc.com/
Other
1.37k stars 113 forks source link

Mismatch in network architecture between original ngp and nerfacc implementation of ngp #152

Closed salykova closed 2 months ago

salykova commented 1 year ago

Dear nerfacc developers,

  1. I noticed that you use #L135 only 15 output values of density mlp (mlp_base) as input to color mlp (mlp_head), whereas in original ngp all 16 outputs are used. May I ask you what was the reason of not using the first output value? Maybe I misunderstood/missed something?

  2. In original ngp there is no activation function after density mlp (mlp_base), whereas you apply exp() function on density

From ngp paper:

The color MLP adds view-dependent color variation. Its input is the concatenation of • the 16 output values of the density MLP, and • the view direction projected onto the first 16 coefficients of the spherical harmonics basis (i.e. up to degree 4). This is a natural frequency encoding over unit vectors.

P.S. I checked github issues, but this wasn't discussed yet. It would be very helpful if you could clarify. Thanks in advance!

liruilong940607 commented 1 year ago

Thanks for the finds out!

I noticed that you use #L135 only 15 output values of density mlp (mlp_base) as input to color mlp (mlp_head), whereas in original ngp all 16 outputs are used. May I ask you what was the reason of not using the first output value? Maybe I misunderstood/missed something?

I didn't put too much efforts on aligning the setting with NGP paper so I think I missed this design. Will try.

In original ngp there is no activation function after density mlp (mlp_base), whereas you apply exp() function on density

I think the NGP uses a relu ac activation (please correct me if I'm wrong). The exp activation is something that we found to be more helpful.

salykova commented 1 year ago

Hi @liruilong940607,

The NGP uses relu activation only on hidden layers and on output of color network (=mlp_head in your case), but there is no activation function for the density network output (=output of mlp_base in your case). So output of mlp_base should be directly feeded into mlp_head without activation. It was discussed here https://github.com/NVlabs/instant-ngp/discussions/167. These are just minor details, I don't think that it will significantly increase/decrease the performance.

liruilong940607 commented 1 year ago

Thanks for the correction! Yeah I also feel it should not affect too much. But great finding!