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

Confusing error message when no optimization package is loaded #68

Closed
salbalkus opened this issue Jul 2, 2024 · 4 comments
Closed

Confusing error message when no optimization package is loaded #68

salbalkus opened this issue Jul 2, 2024 · 4 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@salbalkus
Copy link
Contributor

Consider the following code snippet:

using DensityRatioEstimation
x = randn(100)
y = randn(100) .+ 1
DensityRatioEstimation.fit(KLIEP, x, y, LCV((σ=1,b=50)))

This throws MethodError: no method matching iterate(::Nothing). The actual issue here, it appears, is that the necessary optimization package is not loaded; adding using Optim at the start fixes the problem. However, the error that is thrown obviously does not describe this problem accurately, making the problem difficult to debug for users unfamiliar with how DensityRatioEstimation.jl works.

Perhaps it would be convenient to throw a more descriptive error when this occurs. I have found this package super useful and I think this could be a good quality-of-life addition.

@juliohm
Copy link
Member

juliohm commented Jul 2, 2024

Thank you for reporting the issue @salbalkus. Would you like to submit a PR that adds a more descriptive error message? The current error message is "not implemented", and you can find it here:

_kliep_coeffs(K_nu, K_de, dre::KLIEP, optlib::Type{<:OptimizationLibrary}) = @error "not implemented"

A more descriptive message would include a note to the different optimization libs available for KLIEP, which must be loaded to trigger implementations.

@juliohm juliohm added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jul 2, 2024
@juliohm
Copy link
Member

juliohm commented Jul 4, 2024

@salbalkus need some help to prepare a pull request?

https://github.com/firstcontributions/first-contributions

@salbalkus
Copy link
Contributor Author

@salbalkus need some help to prepare a pull request?

https://github.com/firstcontributions/first-contributions

Hi Júlio, thank you for the response and helpful link! I am preoccupied today but I can prepare a pull request next week 👍

@juliohm
Copy link
Member

juliohm commented Jul 17, 2024

Fixed on master. Thanks!

@juliohm juliohm closed this as completed Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants