search

sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Update mid-manvr test to use column comparisons and isclose #147

Closed jeanconn closed 4 years ago

jeanconn commented 4 years ago

Update mid-manvr test to use column comparisons and isclose

jeanconn commented 4 years ago

I just tossed this here because the test is failing in my tests in a shiny ska3-dev:


>       assert sts.pformat(max_width=-1, max_lines=-1) == exp
E       AssertionError: assert ['      dates...   NMAN', ...] == ['      dates...   NMAN', ...]
E         At index 0 diff: '      datestart              datestop             pitch        pcad_mode' != '      datestart              datestop           pitch     pcad_mode'
E         Use -v to get the full diff

kadi/commands/tests/test_states.py:1227: AssertionError
taldcroft commented 4 years ago

I'm concerned about the rather large matching tolerance of 0.01 deg. That is much more than a numerical issue, so I'd like to see exactly what the diffs are and investigate the source of discrepancy.

jeanconn commented 4 years ago

What tolerance would you like? I thought the test should be verifying pitch was within reasonable tolerances for use in the thermal model so just picked a tolerances as such. If you'd just like it to the 9 printed decimals you had that is fine too.

taldcroft commented 4 years ago

This and many other Ska regression tests also serve the important purpose of validating that we can change numpy version or other core packages without changing downstream program behavior at all. This is how we can justify the shiny branch. So ideally we would like the answers to be the same to binary precision (around 2**-52 or 1e-15 in the mantissa), but in practice that is likely to fail so we can check at something like atol of 1e-8 here.

The key thing is to write tests that are as strict as possible and so we have visibility into regressions (often unrelated to the package itself being tested) and can then understand the issue and whether it might impact other code where testing is not as complete.

taldcroft commented 4 years ago

BTW was the change just a slight formatting diff in the table output or a real numerical change?

jeanconn commented 4 years ago

I thought it was a formatting diff, but of course not easy to see with the previous test.