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

Set AugLagrangian tolerance based on ElemType #217

Merged
merged 2 commits into from Aug 14, 2020
Merged

Conversation

@rcurtin
Copy link
Member

@rcurtin rcurtin commented Aug 12, 2020

Finally, after far too long, I took some time to look into #142. I was able to replicate the problem with the Johnson844LovaszThetaFMatSDP test.

The augmented Lagrangian optimizer (AugLagrangian) keeps a penalty parameter sigma internally. This is stored as a double---but Johnson844LovaszThetaFMatSDP is a test that specifically uses arma::fmat as an objective matrix type (so, e.g., float is the element type).

AugLagrangian was also hard-coded to terminate on an absolute objective difference of 1e-10---but for this particular test, the objective function converges to -14... and due to the precision afforded by 1e-14, it's not actually possible to have two different floats very near -14 that are less than 1e-10 apart! So, I modified the tolerance to be based on the value of numeric_limits<ElemType>::epsilon().

Next, I also set a limit on how large sigma can grow. The optimization will terminate when sigma is greater than half the size that can be represented by ElemType. This issue (when paired with the other one) is what caused these errors:

Parameter 4 to routine SLASCL was incorrect
Parameter 5 to routine SLASCL was incorrect

Those were caused by a too-large sigma.

Anyway, to the best of my belief this fixes the issue. The only thing I am not sure of is why this failure only seemed to occur on an i386 system, instead of also on an x86_64 system. But, I don't know that I'll dig deeper this time---if it works, it works. :)

@zoq
zoq approved these changes Aug 12, 2020
Copy link
Member

@zoq zoq left a comment

Wow, nice catch.

@mlpack-bot
mlpack-bot bot approved these changes Aug 13, 2020
Copy link

@mlpack-bot mlpack-bot bot left a comment

Second approval provided automatically after 24 hours. 👍

@zoq zoq merged commit 88879f3 into mlpack:master Aug 14, 2020
5 checks passed
5 checks passed
Memory Checks Build finished.
Details
Static Code Analysis Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
mlpack master build test Build finished.
Details
@rcurtin rcurtin deleted the rcurtin:auglag-float-fix branch Aug 14, 2020
@rcurtin
Copy link
Member Author

@rcurtin rcurtin commented Aug 14, 2020

Thanks @conradsnicta, those are some nice resources. I think maybe that is what was going on here. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.