update TRDH to use diagonal QN model #132
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #132 +/- ##
==========================================
- Coverage 61.62% 61.45% -0.17%
==========================================
Files 11 11
Lines 1295 1292 -3
==========================================
- Hits 798 794 -4
- Misses 497 498 +1 ☔ View full report in Codecov by Sentry. |
|
Here are the |
| @@ -186,8 +186,9 @@ function TR( | |||
| set_radius!(ψ, ∆_effective) | |||
| subsolver_options.Δk = ∆_effective / 10 | |||
| subsolver_options.ν = ν | |||
| subsolver_args = subsolver == TRDH ? (SpectralGradient(1 / ν, f.meta.nvar),) : () | |||
There was a problem hiding this comment.
Does this imply that we can only use TR-TRDH with SpectralGradient?
Also, I think the operator should be preallocated outside the while loop to facilitate the use of in-place subsolvers (once in-place methods are implemented for all the subsolvers in the same manner as R2!)
There was a problem hiding this comment.
Yes, for the moment, it’s the only option (the other two don’t perform well anyways). I plan to make this an option when I implement the solver object, so I’m not making an effort to preallocate at this point.
There was a problem hiding this comment.
The others perform well on some problems, like SVM for example. I'll keep a branch on my fork then if want still want to be able to produce the results of indef-pg.
There was a problem hiding this comment.
I’m planning to work on the solvers soon, so the option should be in place in the near future.
There was a problem hiding this comment.
I added a note about that in #129.
No description provided.