Jashcraf / katsu

Polarimetric Data Reduction and machine control for measuring the polarization of observatories
https://katsu.readthedocs.io
MIT License
3 stars 0 forks source link

23 tests for hot swappable numpy backends demo use #24

Closed kenjim21 closed 3 months ago

kenjim21 commented 3 months ago

added jax backend compatibility for mueler functions and corresponding testing

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 38.70968% with 76 lines in your changes missing coverage. Please review.

Project coverage is 74.35%. Comparing base (3b8fdc3) to head (b6797eb).

Files Patch % Lines
tests/test_mueller.py 37.83% 69 Missing :warning:
tests/test_katsu_math.py 46.15% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #24 +/- ## =========================================== - Coverage 98.38% 74.35% -24.04% =========================================== Files 3 3 Lines 186 308 +122 =========================================== + Hits 183 229 +46 - Misses 3 79 +76 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jashcraf commented 3 months ago

Ah looks like adding Jax as an optional dependency and not explicitly testing it hurts code coverage. I think that's fine for now, but just writing a note here to consider a way of structuring a test file to maintain the code coverage. Maybe removing it from the automated codecov reporting or just configuring the tests on the remote machine

kenjim21 commented 3 months ago

For the codecov we can def look at adding all the possible autodiff libraries that are being supported to a special designation for testing so that while it's not on the requirements.txt, it'll be install for the automated testing

Jashcraf commented 3 months ago

Oh man did it run with the test uncommented?

kenjim21 commented 3 months ago

Screenshot 2024-08-16 180213

This is the margin of difference between the matrices when I print them

Jashcraf commented 3 months ago

So that value in the lower-left is concerning, but not huge - can you make an issue to investigate this difference? For now I think this is fine for the PR

Jashcraf commented 3 months ago

3…2…1…

LGTM!

kenjim21 commented 3 months ago

Ya i'll add an issue. it looks like for any of the non zero values minus the top row there's its almost consistently off at around the 6th decimal or further