Sanity Checks for Params

[lruthotto]

I'd like to get your thoughts (or even help) on some changes that, I think, would improve the usability of our code. I found that we almost never prevent the user from things like putting into a negative maxStep in an InverseParam. I just did (not consciously) and found that it is very hard to trace back this error.

To prevent some of these things in the future, I was thinking about putting some sanity checks when constructing our types. What do you think?

Concretely, I suggest we check that in InverseParam

boundsLow .<= boundsHigh
alpha > 0 
length(mref) == MInv.nc
maxStep > 0 

maybe more? do you see other places where this might be useful?

[erantreister]

Sure. These can be some helpful tests in the getInverseParam constructor. I would not put the length(mref) == MInv.nc test, though. I'm not sure that it holds for me, for example. Maybe you meant sigmaBack instead of mref? This issue is easy to track anyway, and will cause an error quite fast. You're right regarding the rest. Other checks:
pcgMaxIter >0
1 > pcgTol > 0,
minUpdate > 0,
maxIter >= 0

[lruthotto]

yes, you're right about the length of mref. However, this can be painful, since it will only be observed after computing the misfit (which might take very long). Shall we check that

length(boundsLow)==length(mref)
length(boundsHigh)==length(mref)

the starting guess in GN should be the same size as boundsLow (or it'll crash before any computation is done).

[erantreister]

Not good either. mref is not the starting guess. mref is the reference for the regularization. mref can be a matrix if many regularization terms are used (you made that change). I think that length(boundsHigh)==size(mref,1) should be OK for now, but with some crazy regularizers and model functions this may be wrong too.
How about simply replacing the positions of computeMisfit and computeRegularizer in GN? Do the regularizer first....

[lruthotto]

OK, I think changing the positions of Misfit and Regularizer is a good option for this. For the other checks this discussion seems relevant: https://groups.google.com/forum/#!topic/julia-users/JjT49BGOL2I

[erantreister]

Sounds good. Please, if you haven't started, wait with the push until I
push the barrier GN.

On Thu, Apr 28, 2016 at 8:57 AM, Lars Ruthotto notifications@github.com
wrote:

OK, I think changing the positions of Misfit and Regularizer is a good
option for this. For the other checks this discussion seems relevant:
https://groups.google.com/forum/#!topic/julia-users/JjT49BGOL2I

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#6 (comment)