marcoamonteiro / pi-GAN

415 stars 76 forks source link

[No positional encoding for position and direction] #17

Open tomguluson92 opened 3 years ago

tomguluson92 commented 3 years ago

Dear marco, Thanks for open source the code! I am curious about why positional encoding is not involved in pi-GAN's implementation. As in https://github.com/marcoamonteiro/pi-GAN/blob/master/generators/generators.py#L49, could you please be so kind to tell me the reason of removing positional encoding to transformed_points (xyz) and ray_directions (d)? Thank you for your time!

RaymondJiangkw commented 3 years ago

I think it would be better to look at their paper. They use SIREN, which has been demonstrated to be as effective as positional encoding. This discovery has been "officially" acknowledged by original NeRF team in their follow-up paper.

tomguluson92 commented 3 years ago

I think it would be better to look at their paper. They use SIREN, which has been demonstrated to be as effective as positional encoding. This discovery has been "officially" acknowledged by original NeRF team in their follow-up paper.

Thanks!

dichen-cd commented 3 years ago

Hi @RaymondJiangkw ,

I think it would be better to look at their paper. They use SIREN, which has been demonstrated to be as effective as positional encoding. This discovery has been "officially" acknowledged by original NeRF team in their follow-up paper.

I guess "officially acknowledged" is a little bit too strong as a claim, since the SIREN based net is only used for 2D image regression in the paper. For novel view synthesis with NeRF, they still use ReLU MLPs with positional encoding.

RaymondJiangkw commented 3 years ago

Hi, @DeanChan

Hi @RaymondJiangkw ,

I think it would be better to look at their paper. They use SIREN, which has been demonstrated to be as effective as positional encoding. This discovery has been "officially" acknowledged by original NeRF team in their follow-up paper.

I guess "officially acknowledged" is a little bit too strong as a claim, since the SIREN based net is only used for 2D image regression in the paper. For novel view synthesis with NeRF, they still use ReLU MLPs with positional encoding.

Of course, it's strange, haha, and that's the reason why there's a double quote. In the paper of original NeRF team, they acknowledged the effectiveness of SIREN when dealing with the problem of Coordinate-Based Neural Representations, even though they only applied it to 2D image regression, and positional encoding is exactly targeted at solving the same issue.

I'm sorry if you feel my words are not accurate, but what I want to emphasize is that formula-GAN choosing SIREN as an alternative way is plausible and has its theoretical foundation.

Sorry for my informal claim. 😳

dichen-cd commented 3 years ago

Hi @RaymondJiangkw

Many thanks for your quick reply 😄 It's much clearer now.

I just found a repo applying SIREN to NeRF--, which may support the effectiveness of SIREN.

In their implementation, there's no positional encoding for SIREN-based NeRF either. See frameworks.py#L28-L38