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

[fix] simulated anneal always accepts first iter #153

Merged
merged 3 commits into from
Dec 2, 2021
Merged

[fix] simulated anneal always accepts first iter #153

merged 3 commits into from
Dec 2, 2021

Conversation

dariogoetz
Copy link
Contributor

This addresses #150

It appears to me that the initial ArgminIterData returned by the simulated anneal solver's init method does not contain values for param and cost. Hence the executor's state is not being updated upon calling the solver's init method. This leads to the state having infinity as starting cost, which makes any first solution preferable.

I have added param and corresponding initial cost to the solver's init method.

@stefan-k
Copy link
Member

stefan-k commented Dec 2, 2021

Thank you for this PR! This is actually what I also suspected, apologies that I wasn't able to address this myself.

For some reason the CI is broken since yesterday. I'm having the same problem in #152. I will try to have a look hopefully this evening. My suspicion is that something in the images changed.

@dariogoetz
Copy link
Contributor Author

dariogoetz commented Dec 2, 2021

Don't worry, it was not my intention to apply any pressure to you. I just wanted to offer my help with the PR.

Should I update the changelog, too?

@stefan-k
Copy link
Member

stefan-k commented Dec 2, 2021

I did not interpret this as pressure :) Any help is highly appreciated.

You're welcome to update the changelog if you want, otherwise I will update it once I prepare the next release.

Seems like I was able to fix the CI problems. If you merge the current master into your branch then the compilation of openblas-src should work again and the clippy errors should be gone.

@stefan-k stefan-k added this to the v0.5.0 milestone Dec 2, 2021
@dariogoetz
Copy link
Contributor Author

Thanks for fixing the CI. I have updated the changelog, adding a line about the fix.

@stefan-k
Copy link
Member

stefan-k commented Dec 2, 2021

Thanks a lot once more for this PR!

@stefan-k stefan-k merged commit f5ffe75 into argmin-rs:master Dec 2, 2021
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