Skip to content

Renaming IterationsToRemember to HistorySize for L-BFGS learners. - #2894

Merged
rogancarr merged 2 commits into
dotnet:masterfrom
rogancarr:2882_lbfgs_historySize
Mar 9, 2019
Merged

rogancarr merged 2 commits into
dotnet:masterfrom
rogancarr:2882_lbfgs_historySize

Conversation

@rogancarr

Copy link
Copy Markdown
Contributor

This PR renames the L-BFGS parameter IterationsToRemember to HistorySize to be more in line with the common nomenclature. (Previously it was MemorySize.)

Fixes #2882

@Ivanidzo4ka

Copy link
Copy Markdown
Contributor

It's always EntryPointCatalog

@Ivanidzo4ka Ivanidzo4ka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@rogancarr
rogancarr requested a review from artidoro March 9, 2019 00:07
[TGUI(Description = "Memory size for L-BFGS", SuggestedSweeps = "5,20,50")]
[TlcModule.SweepableDiscreteParamAttribute("MemorySize", new object[] { 5, 20, 50 })]
public int IterationsToRemember = Defaults.IterationsToRemember;
public int HistorySize = Defaults.HistorySize;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
public int HistorySize = Defaults.HistorySize;
public int HistoryLength = Defaults.HistorySize;

Size sounds like the number of parameters we can store.

@rogancarr
rogancarr merged commit 4c83066 into dotnet:master Mar 9, 2019
@rogancarr
rogancarr deleted the 2882_lbfgs_historySize branch March 9, 2019 00:59
@rogancarr

Copy link
Copy Markdown
Contributor Author

Oops, just saw your comment. HistorySize is the common nomenclature, and also used by PyTorch.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants