Skip to content

Increase test tolerance#797

Closed
svillemot wants to merge 1 commit into
ORNL:masterfrom
svillemot:i386
Closed

Increase test tolerance#797
svillemot wants to merge 1 commit into
ORNL:masterfrom
svillemot:i386

Conversation

@svillemot

Copy link
Copy Markdown
Contributor

Needed for i386. Most likely needed because of the 80-bit floating point precision on that platform.
See https://buildd.debian.org/status/fetch.php?pkg=tasmanian&arch=i386&ver=8.1-1&stamp=1739206789&raw=0

This patch has been applied to the Debian package.

Needed for i386. Most likely needed because of the 80-bit floating point
precision on that platform.
See https://buildd.debian.org/status/fetch.php?pkg=tasmanian&arch=i386&ver=8.1-1&stamp=1739206789&raw=0
if (std::abs(sum - 96.0 * 512.0 / 27.0) > 10.0 * Maths::num_tol){ // without 10.0 the test fails on dpcpp with error 1.E-12
#else
if (std::abs(sum - 96.0 * 512.0 / 27.0) > Maths::num_tol){
if (std::abs(sum - 96.0 * 512.0 / 27.0) > 2.0 * Maths::num_tol){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IEEE 754 can be deceptively accurate even if it doesn't make much sense on the math side.

I think it is safe to just remove the #ifdef and go with 10.0 * Maths::num_tol in all cases. This tests a rule on an unbounded domain (0, inf), higher rounding error is expected.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this myself, just close the PR if you want me to do it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I let you fix this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svillemot Thanks for putting Tasmanian into the Debian repo.

Let me know if I can do anything else to help. I normally don't backport bugfixes but I can backport both this and he Python install dir to the 8.2 release and make it 8.2.1, let me know if this would be helpful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your offer to help. Merging Debian-specific patches into the GitHub repo is what matters most, because it means I’ll be able to drop those patches for the next release.

In the meantime I don’t really need a new bugfix release, because the patches are already applied to the Debian package, but feel free to make one if you think it can benefit other users outside Debian.

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