Make scipy.optimize warnings optional, formatted #844
Conversation
timemachine/md/minimizer.py
Outdated
| print(f"performing {method} minimization on {n_local} atoms\n(holding the other {n_frozen} atoms frozen)") | ||
| U_0, grad_0 = val_and_grad_fn_bfgs(x_local_0_flat) | ||
| print(f"U(x_0) = {U_0:.3f} kJ/mol") | ||
| print(f"|force(x_0)| = {np.linalg.norm(grad_0):.3f} kJ/mol / nm^2") |
There was a problem hiding this comment.
is the intention to help with debugging the atom that has the highest force? If so, may want norm or argmax over per-atom force norm here (and print the offending atom idx via bad_idx = np.argmax(np.linalg.norm(grad_0, axis=1)) in addition to the grad_0[bad_idx]). Otherwise, if the intention is to check convergence later on, not sure how useful this is since the norm force will pretty much never be zero (or be insanely large).
There was a problem hiding this comment.
Also going to make a mental note that the forces can probably also overflow.
There was a problem hiding this comment.
is the intention to help with debugging the atom that has the highest force?
That wasn't the original intent -- just to provide more context on the other log messages about minimizer progress (did the function value decrease, did the gradient norm decrease)
If so, may want norm or argmax over per-atom force norm here (and print the offending atom idx via bad_idx = np.argmax(np.linalg.norm(grad_0, axis=1)) in addition to the grad_0[bad_idx]).
Will add!
Otherwise, if the intention is to check convergence later on, not sure how useful this is since the norm force will pretty much never be zero (or be insanely large).
Will remove!
There was a problem hiding this comment.
nit: units are determined by the potential function we pass in, but the log messages are specialized to energies in kJ/mol. Should we specify that the potential function should return energies in kJ/mol?
| method = "BFGS" | ||
|
|
||
| if verbose: | ||
| print("-" * 70) |
There was a problem hiding this comment.
Non blocking: in the future we might consider using rich to format messages like this (here could use console.rule)
Adds context for the internal scipy.optimize warnings emitted by
local_minimize(#821, #828), to clarify that the messages apply to initial energy minimization and are not a diagnostic about the broader simulation.(See e.g. the output in cell [27] of https://github.com/mcwitt/timemachine-demo/blob/a4a3809bd289257940ac41d6d564eaab552afbb5/examples/relative-hydration.ipynb for how this can be ambiguous.)