Skip to content
This repository was archived by the owner on Nov 23, 2018. It is now read-only.

Conversation

kortschak
Copy link
Member

Changes discussed in #136 (comment)

@vladimir-ch @btracey Please take a look.

errors.go Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is better, ErrFuncInit or ErrInitFunc? The latter sounds better to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I actually like it without at all, but that's from a perspective of someone who doesn't yet use this package.

@kortschak
Copy link
Member Author

ErrFunc, ErrGrad are nice and short, but they suggest that they can be used for any invalid values returned from the objective function at any point while (at least at the moment) they are specific to initial data.

The documentation makes this reasonably clear and if the API does need to change in this regard, leaving the name open makes that change non-breaking.

One type, one Error method.

That essentially take the situation to what is in current master - there is one type with a variety of values.

@btracey
Copy link
Member

btracey commented Sep 29, 2015

I'm thinking now that the whole approach to error handling may be misguided.

  • I'm thinking that Local is the wrong place to do initial value handling. Instead, this should be handled by specific Methods. I would imagine that there are many Methods which can handle a starting place of inf (Nelder Mead may already be able to do it).
  • Either way, I do think there is utility in explicitly declaring that the error occurred in initialization. An initialization error is purely the fault of the user. The specifed problem does not work with the recommended algorithm. An error later in the optimization is either indicative of a problem with the code of the Method, the code of the function (if the gradients are wrong), or some interaction with the method and the function (say the function exhibits numerical noise and so cannot converge as far as requested).

@vladimir-ch
Copy link
Member

I'm thinking that Local is the wrong place to do initial value handling. Instead, this should be handled by specific Methods.

Method.Init() returns an error, so we can move the check there if we want.

An initialization error is purely the fault of the user. The specifed problem does not work with the recommended algorithm.

Yes, but the reason might be slightly different: in spite of assumptions the function is not defined everywhere and the user just provided a bad starting point. It is not so difficult to make it happen. For example, functions.HelicalValley panics for points on the plane x = 0 (it should probably return a NaN instead) and the zero vector is such a tempting starting location. Still, it is the fault of the user.

Anyway, we can postpone this discussion and continue it later. I like what is in this PR now, so LGTM. Thanks, @kortschak and sorry for the detour.

@btracey
Copy link
Member

btracey commented Sep 29, 2015

Sgtm. I'll submit a follow-up
On Sep 28, 2015 7:08 PM, "Vladimír Chalupecký" notifications@github.com
wrote:

I'm thinking that Local is the wrong place to do initial value handling.
Instead, this should be handled by specific Methods.

Method.Init() returns an error, so we can move the check there if we want.

An initialization error is purely the fault of the user. The specifed
problem does not work with the recommended algorithm.

Yes, but the reason might be slightly different: in spite of assumptions
the function is not defined everywhere and the user just provided a bad
starting point. It is not so difficult to make it happen. For example,
functions.HelicalValley panics for points on the plane x = 0 (it should
probably return a NaN instead) and the zero vector is such a tempting
starting location. Still, it is the fault of the user.

Anyway, we can postpone this discussion and continue it later. I like what
is in this PR now, so LGTM. Thanks, @kortschak
https://github.com/kortschak and sorry for the detour.


Reply to this email directly or view it on GitHub
#138 (comment).

@kortschak kortschak merged commit bf347ba into master Sep 29, 2015
@vladimir-ch vladimir-ch deleted the func-grad-errors branch September 30, 2015 02:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants