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.

Sign up for GitHub

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

Jump to bottom

Multi-modal metric for 32-bit float translate tests #67

Merged
merged 10 commits into from
Sep 4, 2024
Merged

Multi-modal metric for 32-bit float translate tests #67

merged 10 commits into from
Sep 4, 2024

Conversation

FlorianDeconinck
Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck commented Aug 27, 2024 •
edited
Loading

Description
We propose an (optional) new metric to clear the translate test in case of 32-bit computation were the current metric seems to fail distinguishing between noise and signal.

The new metric goes as follows. Test passes if

  • No NaNs AND
  • Absolute difference <1e-13 (1e-10 for 32-bit float) OR
  • Relative difference 0.0001% (computation is 0.0001% of the reference) OR
  • ULP < 1 (the error is within the "hardware noise")

As of now this metric does not include overrides. It can be triggered in the test by adding --multimodal_metric

And the output looks like this

image

This PR also brings a much needed clean up of the failure report for the regular metric in translate test, removing the callstack. Looks like this:

image

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@FlorianDeconinck
Attempt to quantify noise (non-binding)
979c965
@FlorianDeconinck
Better error when missing data from the netCDF
6fc7158
@FlorianDeconinck
Multi modal float metric (see comparison.py)
5ce87f1 
Refactor previous metric in `LegacyMetric`
Add `--multimodal_metric` option to translate
Clean up failure triggering stack dumps
@FlorianDeconinck
Clamp f32 to 1e-10
59a529c 
Fix metric to require no NaNs
@FlorianDeconinck
Merge branch 'develop' into feature/translate_test_f32
3f80b64
@FlorianDeconinck FlorianDeconinck changed the title New metric for 32-bit float translate tests Multi-modal metric for 32-bit float translate tests Aug 27, 2024
This was referenced Aug 27, 2024
Merged
Closed
fmalatino
fmalatino previously approved these changes Sep 3, 2024
View reviewed changes
twicki
twicki requested changes Sep 4, 2024
View reviewed changes
ndsl/stencils/testing/test_translate.py Outdated Show resolved Hide resolved
ndsl/testing/comparison.py Show resolved Hide resolved
ndsl/testing/comparison.py Outdated Show resolved Hide resolved
ndsl/testing/comparison.py Outdated Show resolved Hide resolved
ndsl/testing/comparison.py Show resolved Hide resolved
twicki
twicki approved these changes Sep 4, 2024
View reviewed changes
Copy link
Collaborator

@twicki twicki left a comment

Choose a reason for hiding this comment

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

LGTM

@FlorianDeconinck FlorianDeconinck merged commit 7f84b32 into develop Sep 4, 2024
5 checks passed
@FlorianDeconinck FlorianDeconinck deleted the feature/translate_test_f32 branch September 4, 2024 13:06
@FlorianDeconinck FlorianDeconinck mentioned this pull request Dec 11, 2024
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twicki twicki twicki approved these changes

@fmalatino fmalatino fmalatino left review comments

@oelbert oelbert Awaiting requested review from oelbert

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@FlorianDeconinck @twicki @fmalatino