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

Tools for dealing with limited precision #1392

Merged
merged 2 commits into from
Sep 6, 2022

Conversation

aarchiba
Copy link
Contributor

@aarchiba aarchiba commented Aug 30, 2022

As we see in #1389, users can miss or ignore the "limited precision" warning when trying to run PINT with 64-bit longdouble values. This PR tries to make PINT run less badly and signal more loudly in this setting.

  • The test suite will not run and will emit an error message about precision.
  • pint.simulation will make fake TOAs but only good to a few microseconds.
  • Functions that require full precision to run can call require_longdouble_precision() to do the test and get an informative exception. I haven't identified any such functions yet (do Fitters work okay, for some values of "okay"?)

I would appreciate additional testing in a 64-bit-only environment (M1 macs, Windows MSVC).

@aarchiba aarchiba requested a review from paulray August 30, 2022 16:44
@aarchiba
Copy link
Contributor Author

aarchiba commented Aug 30, 2022

In particular, the test suite should refuse to run at all, with a message about precision. I'd appreciate if someone could comment out the line in conftest.py and see what happens when it tries to run the test suite - are there functions where catastrophe occurs and we need to use require_longdouble_precision()?

@aarchiba
Copy link
Contributor Author

Longer-term, we could go through and flag which tests require full precision and disable running of those tests on low-precision machines.

@paulray
Copy link
Member

paulray commented Aug 30, 2022

I tried this on my M1 Mac and it works as advertised. make test fails with a useful error message and the Simulate_and_make_MassMass.py script now runs fine. It warns of limited precision in the first cell, but otherwise seems to work great. This seems good. We definitely do want to make sure that there are plenty of warnings when running dangerous things without extended precision, but much stuff will work just fine.

)


def update_fake_toa_clock(ts, model, include_bipm=False, include_gps=True):
"""Update the clock settings (corrections, etc) for fake TOAs
"""Update the clock s:qettings (corrections, etc) for fake TOAs
Copy link
Member

Choose a reason for hiding this comment

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

Typo

@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Merging #1392 (6fbc3e6) into master (0d6aeaf) will decrease coverage by 0.00%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##           master    #1392      +/-   ##
==========================================
- Coverage   62.25%   62.24%   -0.01%     
==========================================
  Files          89       89              
  Lines       20267    20278      +11     
  Branches     3653     3657       +4     
==========================================
+ Hits        12617    12623       +6     
- Misses       6860     6862       +2     
- Partials      790      793       +3     
Impacted Files Coverage Δ
src/pint/pulsar_mjd.py 77.31% <ø> (ø)
src/pint/simulation.py 78.14% <40.00%> (-1.45%) ⬇️
src/pint/utils.py 57.24% <71.42%> (+0.17%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@paulray paulray merged commit 8019b0e into nanograv:master Sep 6, 2022
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