Skip to content

Use full double precision in MFILE output#3130

Merged
timothy-nunn merged 2 commits intomainfrom
3129-use-full-double-precision-in-mfiledat-output
May 1, 2024
Merged

Use full double precision in MFILE output#3130
timothy-nunn merged 2 commits intomainfrom
3129-use-full-double-precision-in-mfiledat-output

Conversation

@jonmaddock
Copy link
Copy Markdown
Contributor

Use full precision of double float in MFILE output; required for precise solution vectors and reliability analysis studies.

@jonmaddock jonmaddock linked an issue Apr 8, 2024 that may be closed by this pull request
@jonmaddock jonmaddock force-pushed the 3129-use-full-double-precision-in-mfiledat-output branch 2 times, most recently from 9ea654d to e0bc01f Compare April 11, 2024 16:06
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

@jonmaddock I'll approve this but won't merge it since you have not yet requested review. Feel free to merge it.

@jonmaddock jonmaddock force-pushed the 3129-use-full-double-precision-in-mfiledat-output branch from c23e3ad to c166e14 Compare April 24, 2024 14:55
@jonmaddock
Copy link
Copy Markdown
Contributor Author

Hi Tim, I've made some changes regarding the regression tolerance that I think are worth you reviewing. Because we've now got full precision in the MFILE.DAT output, there's some floating-point discrepancy between running the regression test locally and on the CI. So the reference (run on the CI (tracker)) would disagree with local (explicitly-requested 0% tolerance) running due to full precision assertions. I've decided to go back to pytest's approx() for this, as it contains clear and proper handling for values close to 0 by using a combination of relative and absolute checks with sensible defaults.

@jonmaddock jonmaddock requested a review from timothy-nunn April 24, 2024 15:49
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

This new method does not appear to respect the --reg-tolerance as exactly as it should.

E.g. I made changes to Cost c25 in the source by multiplying the output value to check the test suite respected the default 5% tolerance. The old code would error as soon as I multiplied by anything ober 1.05, e.g. 1.051.

However, the new code using approx does not fail when multiplying by 1.051, 1.052... it fails at 1.053.

(note I did these tests using both a remote reference MFile and by setting a local one to be the reference: reference_mfile_location = INPUT_FILES_FOLDER / "large_tokamak.MFILE.DAT").

Causes failures due to floating-point discrepancies between local and CI.

Use pytest.approx for sensible relative and absolute difference assertions.
@jonmaddock jonmaddock force-pushed the 3129-use-full-double-precision-in-mfiledat-output branch from 1b8e856 to 7996f55 Compare April 26, 2024 14:12
@jonmaddock
Copy link
Copy Markdown
Contributor Author

Thanks for testing this! I think the reason was due to me using pytest.approx() on the observed (new) value, rather than the reference (old) one; the relative tolerance was being calculated relative to the observed value, not the reference. I've swapped the two around and convinced myself with an example, but you may want to check again to be sure.

@jonmaddock jonmaddock requested a review from timothy-nunn April 26, 2024 14:29
@timothy-nunn timothy-nunn merged commit 5a815ec into main May 1, 2024
@timothy-nunn timothy-nunn deleted the 3129-use-full-double-precision-in-mfiledat-output branch May 1, 2024 09:03
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.

Use full double precision in MFILE.DAT output

2 participants