ACEsuit / IPFitting.jl

Fitting of NBodyIPs
Other
5 stars 10 forks source link

solver argument consistency #21

Open bernstei opened 2 years ago

bernstei commented 2 years ago

It would be nice if different solvers agreed on whether multiple numerical arguments should be passed as further elements of solver[2...] or as an array entirely contained in one element of solversolver[2][...]

This is an example of the latter https://github.com/ACEsuit/IPFitting.jl/blob/87dbe63fe8688b821a21aaf452dd50eda905632d/src/lsq.jl#L451 while :rrqr_lap a few lines below is the former.

I guess in principle a solver might need (conceptually) a single value and an unknown (at coding time) number of values, which might best be encoded as [ val, [ subval1, subval2 ... ] ], but that doesn't actually seem to be the case so far.

cortner commented 2 years ago

I think we need a well-defined interface for different solvers. E.g.,

QR()
RRQR(tol = ...)
LSQR(tol = ..., damp = ... )
LBFGS(history = ..., tol = ... )
ADAM( .... )

and then use dispatch to call the right functions. This will make it extendable and more disciplined.

My only question is whether it is worth doing this here and now for IPFitting or whether it should be done for the rewrite.

bernstei commented 2 years ago

In some ways that'd be even less convenient for me (namely accessibility from the command line/wrapper), but if it's more consistent, I won't complain.

cortner commented 2 years ago

The point is you can now use kwargs even from the command line or f youbwish

bernstei commented 2 years ago

That's true for the arguments, but as you wrote it up there you have to have an if else if ... for the actual choice of function to call. I guess there's probably some version of eval() that could be used.

cortner commented 2 years ago

there is no need for if-else. We just use a bit of meta-programming + dispatch.

bernstei commented 2 years ago

I wasn't sure - some people shy away from running arbitrary functions specified in user string input.