autonomousvision / factor-fields

[SIGGRAPH 2023] We provide a unified formula for neural fields (Factor Fields) and a novel dictionary factorization (Dictionary Fields)
https://apchenstu.github.io/FactorFields/
MIT License
201 stars 10 forks source link

set_optimizable wrong param? #17

Closed Ska-p closed 1 week ago

Ska-p commented 1 month ago

in reconstruction function, inside train_across_scene.py while iterating through the steps_inner value, it is written when satysfying the condition j%steps_inner==steps_inner-3

model.set_optimizable['mlp', 'basis', 'renderer'], True)

but mlp is not a parameter in the set_optmizable function

Should it be model.set_optimizable(['proj','basis','renderer'], True) ?

Kilichbek commented 3 weeks ago
Ska-p commented 3 weeks ago

I'm working with 3D reconstruction. I've changed the set_optimizable function to the following:

    def set_optimizable(self, items, status):
        for item in items:
            if item == 'basis' and self.cfg.model.basis_type != 'none':
                self.basises.requires_grad_(requires_grad=status)
            elif item == 'coeff' and self.cfg.model.coeff_type != 'none':
                self.coeffs.requires_grad_(requires_grad=status)
            elif item == 'proj':
                self.linear_mat.requires_grad_(requires_grad=status)
            elif item == 'renderer':
                self.renderModule.requires_grad_(requires_grad=status)

In the older version, when the method entered the 'coeff' elif, it was actually working on the basises module so basically the coefficient where never stopped from updating (in your code the problem is still there). After changing the function I tracked with pdb.set_trace() and apparently the function requiresgrad(value) correctly sets the value as the argument. I'm having as well worse results than the one stated in the paper but they are close.

Update: I checked as well the values of the parameters in the scenario where basises, proj and render do NOT have to be optimized. With my settings the values remains frozen so i believe it should be ok like this.