Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix \phi_0 and d\phi_0 values #118

Merged
merged 4 commits into from
Jun 5, 2018
Merged

Fix \phi_0 and d\phi_0 values #118

merged 4 commits into from
Jun 5, 2018

Conversation

anriseth
Copy link
Collaborator

@anriseth anriseth commented Jun 5, 2018

The values of ϕ_0 and dϕ_0 are ϕ(0), dϕ(0), not ϕ(\alpha_0)).

It is a bit confusing to use α_0 as the initial guess, because it means that we think ϕ_0 refers to the value at the initial guess. Maybe we should change all α0 / α_0's to αinitial?

@codecov
Copy link

codecov bot commented Jun 5, 2018

Codecov Report

Merging #118 into master will decrease coverage by 0.06%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
- Coverage   60.68%   60.62%   -0.07%     
==========================================
  Files           8        8              
  Lines         646      645       -1     
==========================================
- Hits          392      391       -1     
  Misses        254      254
Impacted Files Coverage Δ
src/backtracking.jl 81.57% <0%> (ø) ⬆️
src/morethuente.jl 55.08% <50%> (+0.05%) ⬆️
src/hagerzhang.jl 52.42% <75%> (-0.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64f0414...a323bae. Read the comment docs.

@anriseth anriseth merged commit 32a2766 into master Jun 5, 2018
@anriseth anriseth deleted the fixbtinitial branch June 5, 2018 10:05
@pkofod
Copy link
Member

pkofod commented Jun 5, 2018

Hm, I don't think I agree with this change. phi_0 zero is meant to be the initial phi (and dphi as well), so those calculations were just meant to calculate the values if they weren't provided already. Or am I missing something?

@anriseth
Copy link
Collaborator Author

anriseth commented Jun 5, 2018

No, phi_0 and dphi_0 that the line stretch algorithms use are the values of phi at alpha=0. That is what Optim passes to the line searches.

@pkofod
Copy link
Member

pkofod commented Jun 5, 2018

Ah, right, sorry, it's the numbers used to check armijo/wolfe/.../conditions ... good catch :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants