-
Notifications
You must be signed in to change notification settings - Fork 20
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
modified accelerator for variable timestep #341
Conversation
03e08e4
to
b6259ca
Compare
b6259ca
to
bdbb9e0
Compare
Looks great! Just two things:
|
Hi Eviatar, Great points
From the calculations leading to Eqn 4.41 in https://arxiv.org/pdf/2209.11371.pdf we have that the ensemble approximation is exactly addressing the gradient of the misfit However if we follow through the discretization, we can bridge from 4.41 to our code via |
5d36633
to
7ac2e4d
Compare
ce0d3d5
to
ea5c26d
Compare
LGTM after you run the autoformatter! |
bors r+ |
341: modified accelerator for variable timestep r=odunbar a=odunbar <!--- THESE LINES ARE COMMENTED --> ## Purpose <!--- One sentence to describe the purpose of this PR, refer to any linked issues: #14 -- this will link to issue 14 Closes #2 -- this will automatically close issue 2 on PR merge --> - closes #339 ## Content <!--- specific tasks that are currently complete - Solution implemented --> - changes the momentum parameter from $\beta_k = \frac{2}{k+2}$ into $\beta_k = \theta_k(\theta_{k-1}^{-1} - 1)$ with $\theta_{0} = 1$. The new general formulation always satisfies a required quadratic inequality needed for convergence acceleration proofs. Explicitly, one picks $\theta_k$ to satisfy $\frac{1-\theta_k}{\theta_k^2}\Delta t_k \leq \frac{\Delta t_{k-1}}{\theta_{k-1}^2}$. - small UKI bug-fix (just stops some warnings on compile) - moved the original `NesterovAccelerator` to `ConstantStepNesterovAccelerator` ## Results from Unit tests Runs additional tests with 20 steps of the `DataMisfitController()` learning rate scheduler. Preliminary findings - It seems even for constant timesteps this formulation is better (in the early stages) than the original formulation - It seems that the update seems to give benefit to all of the methods using DMC timestepper/ EKS Stable timestepper. ### For Constant timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.507262042724763 Accelerator: ConstantStepNesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 17.64047643331899 Accelerator: NesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.059129584415419 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: ConstantStepNesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.394047688287434 Accelerator: NesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.39590620316884 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.537821536383912 Accelerator: ConstantStepNesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 11.155144156229342 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.574272962450679 ``` ### For variable timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.883305568165869 Accelerator: NesterovAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.4114237694466025 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: NesterovAccelerator Process: TransformInversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.400576672334215 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.765752171697719 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.2125091342967265 Accelerator: DefaultAccelerator Process: Sampler ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 6.239456914512827 Accelerator: NesterovAccelerator Process: Sampler ┌ Warning: Acceleration is experimental for Sampler processes and may affect convergence. └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/EnsembleKalmanProcess.jl:220 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.896253011160538 ``` <!--- Review checklist I have: - followed the codebase contribution guide: https://clima.github.io/ClimateMachine.jl/latest/Contributing/ - followed the style guide: https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/ - followed the documentation policy: https://github.com/CliMA/policies/wiki/Documentation-Policy - checked that this PR does not duplicate an open PR. In the Content, I have included - relevant unit tests, and integration tests, - appropriate docstrings on all functions, structs, and modules, and included relevant documentation. --> ---- - [x] I have read and checked the items on the review checklist. Co-authored-by: odunbar <odunbar@caltech.edu>
Timed out. |
bors r+ |
341: modified accelerator for variable timestep r=odunbar a=odunbar <!--- THESE LINES ARE COMMENTED --> ## Purpose <!--- One sentence to describe the purpose of this PR, refer to any linked issues: #14 -- this will link to issue 14 Closes #2 -- this will automatically close issue 2 on PR merge --> - closes #339 ## Content <!--- specific tasks that are currently complete - Solution implemented --> - changes the momentum parameter from $\beta_k = \frac{2}{k+2}$ into $\beta_k = \theta_k(\theta_{k-1}^{-1} - 1)$ with $\theta_{0} = 1$. The new general formulation always satisfies a required quadratic inequality needed for convergence acceleration proofs. Explicitly, one picks $\theta_k$ to satisfy $\frac{1-\theta_k}{\theta_k^2}\Delta t_k \leq \frac{\Delta t_{k-1}}{\theta_{k-1}^2}$. - small UKI bug-fix (just stops some warnings on compile) - moved the original `NesterovAccelerator` to `ConstantStepNesterovAccelerator` ## Results from Unit tests Runs additional tests with 20 steps of the `DataMisfitController()` learning rate scheduler. Preliminary findings - It seems even for constant timesteps this formulation is better (in the early stages) than the original formulation - It seems that the update seems to give benefit to all of the methods using DMC timestepper/ EKS Stable timestepper. ### For Constant timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.507262042724763 Accelerator: ConstantStepNesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 17.64047643331899 Accelerator: NesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.059129584415419 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: ConstantStepNesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.394047688287434 Accelerator: NesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.39590620316884 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.537821536383912 Accelerator: ConstantStepNesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 11.155144156229342 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.574272962450679 ``` ### For variable timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.883305568165869 Accelerator: NesterovAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.4114237694466025 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: NesterovAccelerator Process: TransformInversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.400576672334215 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.765752171697719 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.2125091342967265 Accelerator: DefaultAccelerator Process: Sampler ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 6.239456914512827 Accelerator: NesterovAccelerator Process: Sampler ┌ Warning: Acceleration is experimental for Sampler processes and may affect convergence. └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/EnsembleKalmanProcess.jl:220 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.896253011160538 ``` <!--- Review checklist I have: - followed the codebase contribution guide: https://clima.github.io/ClimateMachine.jl/latest/Contributing/ - followed the style guide: https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/ - followed the documentation policy: https://github.com/CliMA/policies/wiki/Documentation-Policy - checked that this PR does not duplicate an open PR. In the Content, I have included - relevant unit tests, and integration tests, - appropriate docstrings on all functions, structs, and modules, and included relevant documentation. --> ---- - [x] I have read and checked the items on the review checklist. Co-authored-by: odunbar <odunbar@caltech.edu>
Timed out. |
simplify expression format rm typo in UKI that causes warning needs a sqrt remove r add ref into docstring re-add original nesterov scheme as ConstantStepNesterovAccelerator for comparisons format github actions takes ages, processing bug I expect, increasing timeout
9edcb0d
to
385a04c
Compare
bors r+ |
341: modified accelerator for variable timestep r=odunbar a=odunbar <!--- THESE LINES ARE COMMENTED --> ## Purpose <!--- One sentence to describe the purpose of this PR, refer to any linked issues: #14 -- this will link to issue 14 Closes #2 -- this will automatically close issue 2 on PR merge --> - closes #339 ## Content <!--- specific tasks that are currently complete - Solution implemented --> - changes the momentum parameter from $\beta_k = \frac{2}{k+2}$ into $\beta_k = \theta_k(\theta_{k-1}^{-1} - 1)$ with $\theta_{0} = 1$. The new general formulation always satisfies a required quadratic inequality needed for convergence acceleration proofs. Explicitly, one picks $\theta_k$ to satisfy $\frac{1-\theta_k}{\theta_k^2}\Delta t_k \leq \frac{\Delta t_{k-1}}{\theta_{k-1}^2}$. - small UKI bug-fix (just stops some warnings on compile) - moved the original `NesterovAccelerator` to `ConstantStepNesterovAccelerator` - added 1 hr to bors timer, mac OS tests are being 10x slower today than yesterday (with no changes on our end) ## Results from Unit tests Runs additional tests with 20 steps of the `DataMisfitController()` learning rate scheduler. Preliminary findings - It seems even for constant timesteps this formulation is better (in the early stages) than the original formulation - It seems that the update seems to give benefit to all of the methods using DMC timestepper/ EKS Stable timestepper. ### For Constant timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.507262042724763 Accelerator: ConstantStepNesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 17.64047643331899 Accelerator: NesterovAccelerator Process: Inversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.059129584415419 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: ConstantStepNesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.394047688287434 Accelerator: NesterovAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.39590620316884 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.537821536383912 Accelerator: ConstantStepNesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 11.155144156229342 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.574272962450679 ``` ### For variable timestep problems ``` Accelerator: DefaultAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.883305568165869 Accelerator: NesterovAccelerator Process: Inversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.4114237694466025 Accelerator: DefaultAccelerator Process: TransformInversion ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.414729659424668 Accelerator: NesterovAccelerator Process: TransformInversion [ Info: Termination condition of timestepping scheme `DataMisfitController` will be exceeded during the next iteration. ┌ Warning: Termination condition of timestepping scheme `DataMisfitController` has been exceeded. Preventing futher updates │ Set on_terminate="continue" in `DataMisfitController` to ignore termination └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/LearningRateSchedulers.jl:278 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.400576672334215 Accelerator: DefaultAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.765752171697719 Accelerator: NesterovAccelerator Process: Unscented ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 5.2125091342967265 Accelerator: DefaultAccelerator Process: Sampler ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 6.239456914512827 Accelerator: NesterovAccelerator Process: Sampler ┌ Warning: Acceleration is experimental for Sampler processes and may affect convergence. └ @ EnsembleKalmanProcesses ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/src/EnsembleKalmanProcess.jl:220 ┌ Info: Convergence: │ cost_initial = 123.31914542296597 └ cost_final = 4.896253011160538 ``` <!--- Review checklist I have: - followed the codebase contribution guide: https://clima.github.io/ClimateMachine.jl/latest/Contributing/ - followed the style guide: https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/ - followed the documentation policy: https://github.com/CliMA/policies/wiki/Documentation-Policy - checked that this PR does not duplicate an open PR. In the Content, I have included - relevant unit tests, and integration tests, - appropriate docstrings on all functions, structs, and modules, and included relevant documentation. --> ---- - [x] I have read and checked the items on the review checklist. Co-authored-by: odunbar <odunbar@caltech.edu>
Timed out. |
bors r+ |
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Purpose
Content
NesterovAccelerator
toConstantStepNesterovAccelerator
Results from Unit tests
Runs additional tests with 20 steps of the
DataMisfitController()
learning rate scheduler. Preliminary findingsFor Constant timestep problems
For variable timestep problems