23 tests for hot swappable numpy backends demo use #24
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
|
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 |
katsu/mueller.py
Outdated
|
|
||
| D = np.sqrt(np.sum(diattenuation_vector * diattenuation_vector, axis=-1)) | ||
| mD = np.sqrt(1 - D**2) | ||
| D = np.sqrt(np.sum(np.matmul(diattenuation_vector, diattenuation_vector), axis=-1)) |
There was a problem hiding this comment.
It looks like an element wise multiplication was replaced with a matmul. Does this end up doing a dot product because it does so in the last axis?
There was a problem hiding this comment.
Yeah I think this might need to be reverted to an element-wise multiplication, but let me check the source material first.
There was a problem hiding this comment.
I just tried reverting the change and uncommenting the test for it. still getting a failure unfortunately (though this could still be because of the eigen values thing)
There was a problem hiding this comment.
So decompose_diattenuator doesn’t rely on eigenvalue computation, which suggests that maybe its something significant
There was a problem hiding this comment.
i’ll try poke around on my machine
There was a problem hiding this comment.
If I did it right I think i just commited the change
There was a problem hiding this comment.
okay, let me push uncommenting the test and see if it passes on the workflow
There was a problem hiding this comment.
Yeah for decompose_diattenuator, not for decompose_depolarizer
There was a problem hiding this comment.
wait we still have it not required so it won't check it regardless. can you test on your computer if the tests work now?
There was a problem hiding this comment.
decompose_depolarizer fails if I uncomment it
|
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 |
|
Oh man did it run with the test uncommented? |
|
This is the margin of difference between the matrices when I print them |
|
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 |
|
3…2…1… LGTM! |
|
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 |
kenjim21
deleted the
23-tests-for-hot-swappable-numpy-backends-demo-use
branch
added jax backend compatibility for mueler functions and corresponding testing