mitre / menelaus

Online and batch-based concept and data drift detection algorithms to monitor and maintain ML performance.
https://menelaus.readthedocs.io/en/latest/
Apache License 2.0
66 stars 7 forks source link

Add fixed NNDVI #124

Closed Anmol-Srivastava closed 2 years ago

Anmol-Srivastava commented 2 years ago

Adds

Fixes

Closes #36

Anmol-Srivastava commented 2 years ago

@tms-bananaquit I think you can begin review of this PR while I work out remaining issues with formatting/coverage. The major thing I anticipate you'll notice is the hard-coded unit tests for some of these compute functions (and elsewhere), for which I couldn't really find an alternative for now

tms-bananaquit commented 2 years ago

The sklearn dependency is already listed as part of install_requires, so we shouldn't need to add it to the other settings in setup.cfg. The logs show the build step is pulling python ver 3.10.8 instead of 3.10.7, and the sklearn .tar is a different file, but installation is apparently finishing successfully.

Apparently installing from the sklearn alias has been deprecated for a while, so I switched the dependency to scikit-learn. That fixed the build issue. Now there are some unit tests failing for other modules, which doesn't make a whole lot of sense given what code has(n't) been modified.

Anmol-Srivastava commented 2 years ago

The only full failure I see is the coverage 100% due to un-tested lines in nndvi.py -- addressing that now

Anmol-Srivastava commented 2 years ago

The mu == 0 case was removed because it's effectively no longer possible with the new change (and was cutting warnings in a very unlikely case to begin with). The remaining line to be covered is just the if drift, reset line which for some reason isn't being counted by one of the unit tests.

Anmol-Srivastava commented 2 years ago

Should be ready for review now

Anmol-Srivastava commented 2 years ago

Did the validation / docstring changes, waiting on some example business to resolve that comment too.

Anmol-Srivastava commented 2 years ago

Pushed the NNDVI-updated notebook for the time being, ~having some difficulty checking its look on my end~ the added code segment executes but it's taking a while to get output