Skip to content
This repository was archived by the owner on Apr 1, 2019. It is now read-only.

Fixed unhelpful error message when optim.method is wrong #2

Open
otoomet wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixed unhelpful error message when optim.method is wrong #2

otoomet wants to merge 5 commits into from

Conversation

@otoomet
Copy link

@otoomet otoomet commented Dec 28, 2015

If !(optim.method %in% c("BFGS", "gen") R responded by

Error in matrix(runif(n * param.n), nrow = param.n, ncol = n) : non-numeric matrix extent. methods are handled in lower case now internally

I changed it in the following way:

  • optim.method is converted to lower case internally
  • if the method is unknown, R exits with an informative error message
  • as now also "bfgs" and "GEN" are allowed, this is also mentioned in the documentation

The reasons for these changes are the following:

  • I run into the problem early when I started to use DiceKriging, and struggled some time to understand it.
  • I was also confused by the upper case "BFGS" and lower case "gen" difference.
  • In general, I think the error messages should be helpful and informative.

I agree that the choice to use lower case letters internally can be discussed, and I am happy to do that. Are you the right person for this? Or should I talk to Olivier (CRAN maintainer?) I would be happy to contribute more in polishing the package, but that would mean many decisions to be discussed with the maintainers.

Cheers,
Ott

PS: I had issues with git merging, so I am afraid all the former changes are included here as well.

otoomet and others added 5 commits September 18, 2015 13:27
kmEstimate: corrected an error where if some of the foreach workers
ea3ac7a 
	    give error, the code still attempted to handle full-length
	    return vector 'olist'.
scalingFun example works
9e9c1c7
@otoomet
kmEstimates.R a spacing difference with master fixed
1b5ef98
@otoomet
fixed an error when unknown optim.method resulted in error: Error in …
565574d 
…matrix(runif(n * param.n), nrow = param.n, ncol = n) :

  non-numeric matrix extent.  methods are handled in lower case now internally
@otoomet
lower case optimization methods mentioned in documentation
8ccc5ef
@yannrichet
Copy link
Contributor

yannrichet commented Dec 30, 2015

Hi Ott,

I will try to incluse that soon. I also forward to Olivier for his next
release.

Best

Yann
Le 28 déc. 2015 04:30, "Ott Toomet" [email protected] a écrit :

If !(optim.method %in% c("BFGS", "gen") R responded by

Error in matrix(runif(n * param.n), nrow = param.n, ncol = n) :
non-numeric matrix extent. methods are handled in lower case now internally

I changed it in the following way:

  • optim.method is converted to lower case internally
  • if the method is unknown, R exits with an informative error message
  • as now also "bfgs" and "GEN" are allowed, this is also mentioned in
    the documentation

The reasons for these changes are the following:

  • I run into the problem early when I started to use DiceKriging, and
    struggled some time to understand it.
  • I was also confused by the upper case "BFGS" and lower case "gen"
    difference.
  • In general, I think the error messages should be helpful and
    informative.

I agree that the choice to use lower case letters internally can be
discussed, and I am happy to do that. Are you the right person for this? Or
should I talk to Olivier (CRAN maintainer?) I would be happy to contribute
more in polishing the package, but that would mean many decisions to be
discussed with the maintainers.

Cheers,
Ott

PS: I had issues with git merging, so I am afraid all the former changes

are included here as well.

You can view, comment on, or merge this pull request online at:

#2
Commit Summary

  • kmEstimate: corrected an error where if some of the foreach workers
  • scalingFun example works
  • kmEstimates.R a spacing difference with master fixed
  • fixed an error when unknown optim.method resulted in error: Error in
    matrix(runif(n * param.n), nrow = param.n, ncol = n) :
  • lower case optimization methods mentioned in documentation

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@otoomet @yannrichet