Reformat the gaussian conditional sampler #366
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #366 +/- ##
==========================================
- Coverage 98.10% 97.91% -0.20%
==========================================
Files 22 20 -2
Lines 1161 1053 -108
==========================================
- Hits 1139 1031 -108
Misses 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Joseph Paillard <[email protected]>
Co-authored-by: Joseph Paillard <[email protected]>
| ) | ||
| generator.fit(X=X) | ||
| X_tilde = generator.sample() | ||
| assert X_tilde.shape == (n, p) |
There was a problem hiding this comment.
Sounds like a smoke test.
Maybe you could add assert np.linalg.norm(X - X_tilde) < np.linalg.norm(X) (assuming that this test works)
There was a problem hiding this comment.
This assertion doesn't work.
For the moment, I don't do the test of functionalities. I just have tests for the coverage.
I don't want to over load the Pull Request.
There was a problem hiding this comment.
Then np.linalg.norm(X - X_tilde) < np.sqrt(2) * np.linalg.norm(X) should work
But we need to test that the generated knockoffs are not crazy.
| d = np.eye(n) | ||
| sigma = u * d * u.T | ||
| _s_equi(sigma) | ||
|
|
There was a problem hiding this comment.
I did one test, tell me if it's ok.
There was a problem hiding this comment.
Sorry, I mean that we should either drop lines 62-67 or add a finall assert.
Co-authored-by: bthirion <[email protected]>
I reformat the Gaussian estimation of knockoff in the format of classes, as conditional sampling.
I moved all the conditional samplers into statistical_tool.