ProGamerGov / neural-style-pt

PyTorch implementation of neural style transfer algorithm
MIT License
834 stars 178 forks source link

The num_corrections default value is actually 100, not 0 #7

Closed ajhool closed 4 years ago

ajhool commented 5 years ago

In neural-style-pt, (and the original neural-style.lua, actually), if params.lbfgs_num_corrections is not set, the default is 0 (line 29). Then, the optim.history object is only updated if params.num_corrections > 0:

       #line 56
        if params.lbfgs_num_correction > 0:
            optim_state['history_size'] = params.lbfgs_num_correction

However, the torch.optim.lbfgs class uses a default value of 100 if history_size is not set:

torch.optim.LBFGS(params, lr=1, max_iter=20, max_eval=None, tolerance_grad=1e-05, tolerance_change=1e-09, history_size=100, line_search_fn=None)

See the lbfgs section: https://pytorch.org/docs/stable/optim.html

Thus, if somebody tries to set lbfgs_num_corrections to 0 (the default) to save memory they will actually use size 100.

ajhool commented 5 years ago

For completeness' sake... the fix would be to either set optim.history_size = 0 explicitly, or to change the lbfgs_num_correction default value to 100 to reflect the actual default value used by torch.optim.lbfgs.

ProGamerGov commented 5 years ago

Does neural-style or neural-style-pt work with a history size value of 0? Because I understood that zero was used because it wasn't a valid option in this case.

ajhool commented 5 years ago

From looking at the lbfgs source, I think it would error out. People who are unfamiliar with lbfgs (myself included) might try to "increase" the history size by setting it to 1 or 20 or 50, without realizing that they are actually decreasing the history size from the default of 100. I see it as a consistency thing, especially when setting other parameters like tv_weight to 0 really does disable them. It seems more natural to use a default for lbfgs_num_corrections of 100 if that's actually what the underlying default is

ProGamerGov commented 5 years ago

Upon testing, it seems that a history size of zero does indeed result in an error:

Running optimization with L-BFGS
Traceback (most recent call last):
  File "neural_style.py", line 410, in <module>
    main()
  File "neural_style.py", line 243, in main
    optimizer.step(feval)
  File "/usr/local/lib/python2.7/dist-packages/torch/optim/lbfgs.py", line 147, in step
    old_dirs.pop(0)
IndexError: pop from empty list
ProGamerGov commented 4 years ago

This change has now been implemented in the multi-gpu branch: https://github.com/ProGamerGov/neural-style-pt/pull/20, and it has been implemented in the pip package as well.