313 add timings for gcp opt with l bfgs #314
Conversation
…ace monotonic timer with perf_counter
…sues in mypy surrounding __call__ .
ntjohnson1
left a comment
ntjohnson1
left a comment
There was a problem hiding this comment.
LGTM. I added a PR into your branch here that is one approach to resolving the typing issues specific to this PR.
The other typing issues for python > 3.8 should be resolved here since they are partially related to numpy 2.0 and newer mypy. I didn't check them exhaustively so its possible that there is still some subtle error remaining but resolving those two PRs first should at least highlight what remains before digging too much deeper.
tests/gcp/test_optimizers.py
Outdated
| solver = LBFGSB(maxiter=maxiter) | ||
| _, info = solver.solve(model, dense_data, gaussian, gaussian_grad) | ||
| assert not "callback" in info["callback"].keys() | ||
| assert info["callback"]["time_trace"].shape == (maxiter,) |
There was a problem hiding this comment.
Can we update this to ensure the timings are greater than zero? When looking at your typing bug I accidentally turned off the timing portion but the member in the Monitor had the shape set correctly (but always returned zero)
There was a problem hiding this comment.
Are you asking that the test check that timings > 0? Or something else? I'm having trouble parsing the second sentence.
There was a problem hiding this comment.
Yes. That's the request. Right now if ONLY the constructor for the monitor existed this test would still pass.
There was a problem hiding this comment.
Ah okay, that makes sense.
|
|
||
| # Test default callback | ||
| maxiter = 2 | ||
| solver = LBFGSB(maxiter=maxiter) |
There was a problem hiding this comment.
The code has a comment about re-using the solver but we don't test for it. Can we add a test to make sure that is supported correctly? (My MR to fix the types also changes how we reset things when we are done so a test would confirm our intentions align)
lbfgsb type narrowing
…hub.com/jeremy-myers/pyttb into 313-add-timings-for-gcp_opt-with-l-bfgs
|
Regression tests fail on other code that was already failing, probably related to numpy & mypy in this PR. |
3 participants
LBFGSBcalledMonitortests/gcp/test_optimizers.pymonotonic()toperf_counter()inStochasticSolvergcp_optininfodictionary📚 Documentation preview 📚: https://pyttb--314.org.readthedocs.build/en/314/