Open liv0617 opened 3 months ago
Thanks @liv0617 for the update! I haven't been working on this for a long time now. I'm wondering is there a minimal example to test the effect of this change?
Sorry for missing this @greentfrapp!
--
It's a bit hard to test this because (1) this bug changes model behavior, but doesn't completely break it; and (2) to rigorously test, we would need a model implementation we trust in order to validate it, which we don't have in PyTorch if we don't trust this implementation.
The experiment I did which made me pretty confident the new version is correct (besides inspect InceptionV1's parameters as shown above) was reproducing the curve detectors synthetic data plots form Cammarata et al. https://distill.pub/2020/circuits/curve-detectors/ . By default, the plots are very different. But with this fix, I reproduce the plots.
The
alpha
andk
values forLocalResponseNorm0
andLocalResponseNorm1
are incorrect. The code originally used to constructlucent/modelzoo/inceptionv1/InceptionV1.py
has since been updated to correct this, but that change isn't reflected here.In order to validate the correct values, one can do the following:
Download the original InceptionV1 protobuf file used in Lucid.
Open and inspect the contents:
and then the various attributes of the LocalResponseNorm can be examined. Example output for
localresponsenorm1
:LocalResponseNorm layers differ slightly in TensorFlow and in PyTorch: