aleximmer / Laplace

Laplace approximations for Deep Learning.
https://aleximmer.github.io/Laplace
MIT License
436 stars 63 forks source link

Failing tests: KFACLinearOperator object has no attribute _mapping #150

Closed elcorto closed 4 months ago

elcorto commented 4 months ago

Hi

Thanks for this package. I have been testing the current main (fd4de8a8a2585230d1c276c32bd50307fa7e2276) with support for curvlinops. While examples/regression_example.py runs, my code and many (all?) KronLaplace tests fail with

laplace/baselaplace.py:907: in fit
    super().fit(train_loader, override=override)
laplace/baselaplace.py:410: in fit
    loss_batch, H_batch = self._curv_closure(X, y, N)
laplace/baselaplace.py:887: in _curv_closure
    return self.backend.kron(X, y, N=N)
laplace/curvature/curvlinops.py:72: in kron
    kron = self._get_kron_factors(linop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <laplace.curvature.curvlinops.CurvlinopsGGN object at 0x2aab7f54f850>
linop = <122x122 KFACLinearOperator with dtype=float32>

    def _get_kron_factors(self, linop):
        kfacs = list()
        for name, module in self.model.named_modules():
>           if name not in linop._mapping.keys():
E           AttributeError: 'KFACLinearOperator' object has no attribute '_mapping'

laplace/curvature/curvlinops.py:39: AttributeError

(full log attached).

System info:

log.txt

runame commented 4 months ago

Hi Steve,

Can you clone the curvlinops repo and install it from the main branch? This should resolve the issue.

We depend on the latest version of curvlinops that is not available on pypi yet. Sorry for the confusion, we are currently making major upgrades to our library and the main branch might be unstable for bit (but you'll get many new features).

elcorto commented 4 months ago

Hi Steve,

Can you clone the curvlinops repo and install it from the main branch? This should resolve the issue.

That works, thanks.

We depend on the latest version of curvlinops that is not available on pypi yet. Sorry for the confusion, we are currently making major upgrades to our library and the main branch might be unstable for bit (but you'll get many new features).

No worries, I wasn't using a released version so I don't expect things to be super stable. Having said that, since the latest tag 0.1a2 is quite old, I was using the previous main (f6af73668870834eee15893c27a250c71966495c) so far as quasi-stable version and after you updated that with new features, I gave it a shot.

Thanks for your work!

wiseodd commented 4 months ago

Thanks for trying the new features so quickly! I created a fix for this confusion in #151.

Stay tuned for a new release later in several months! We'll do so once #144, #145, and #148 are merged to the master branch.

elcorto commented 4 months ago

Thanks. I'm aware of the sharp edges and will test main along with main of all deps should problems arise.