Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace invalidArg with invalid_arg #153

Merged
merged 1 commit into from
Mar 2, 2016
Merged

Replace invalidArg with invalid_arg #153

merged 1 commit into from
Mar 2, 2016

Conversation

rleonid
Copy link
Owner

@rleonid rleonid commented Mar 1, 2016

Give more power about specifying errors. I'm not ready to convert all partial functions to use an option or result type (not certain if we'll go that route), but wanted more capabilities with reporting errors.
@struktured or @ihodes any opinions?

Give more power about specifying errors.
@struktured
Copy link
Contributor

This is a good PR. I think adopting a result or or_error type is interesting if done appropriately. For the most part I would definitely go beyond optional if you are going to bother at all encoding the errors in return types.

The strictness and explicitness of the error handling should correspond to how much we would expect a particular function to "misbehave", and thus will probably need to vary by module and/or function signature. In general it's probably best to prefer implicit error handling so mathematicians can focus on the meat of their computations, but I'm open to counter arguments.

For instance, it would be nice to know when training a model that some particular hyperparameter was invalid or led to non-convergence- we can then condition on it that information and try different inputs. More generally, you could imagine any function reporting what input parameter or derived value (with semantically meaningful name) broke the results.

Anyhow, I'd just merge this PR in. looks good to go.

@rleonid rleonid mentioned this pull request Mar 2, 2016
@rleonid
Copy link
Owner Author

rleonid commented Mar 2, 2016

Awesome, I'm adding an issue where we can track the discussion.

rleonid added a commit that referenced this pull request Mar 2, 2016
Replace invalidArg with invalid_arg
@rleonid rleonid merged commit c3085b0 into master Mar 2, 2016
@rleonid rleonid deleted the invalid_arg_ftw branch March 2, 2016 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants