Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added text for drift differentiation #1815

Merged
merged 5 commits into from
Jul 26, 2022
Merged

Added text for drift differentiation #1815

merged 5 commits into from
Jul 26, 2022

Conversation

nirhutnik
Copy link
Contributor

(For now only added paragraph in drift guide and TrainTestFeatureDrift - will add for the other relevant checks when we agree on how to phrase it).

@nirhutnik nirhutnik added the documentation modification of the documentation / readme's label Jul 24, 2022
@nirhutnik nirhutnik added this to the Darwin milestone Jul 24, 2022
@nirhutnik nirhutnik self-assigned this Jul 24, 2022
@nirhutnik nirhutnik requested review from ItayGabbay, shir22 and a team as code owners July 24, 2022 16:26
Copy link
Collaborator

@noamzbr noamzbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved assuming update to the train_test_feature_drift check as well

As mentioned above, we recommend to use either `Cramer's V <https://en.wikipedia.org/wiki/Cram%C3%A9r%27s_V>`__ or
`PSI <https://www.lexjansen.com/wuss/2017/47_Final_Paper_PDF.pdf>`__ for categorical variables, and use Cramer's V by default.
PSI is widely used in the industry, but does not have an upper limit and is not very explainable.
Cramer's V is always in the range [0,1], and can be interpreted as the correlation between the variable's distribution and the dataset (train or test).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last part isn't clear

docs/source/user-guide/general/drift_guide.rst Outdated Show resolved Hide resolved
docs/source/user-guide/general/drift_guide.rst Outdated Show resolved Hide resolved
docs/source/user-guide/general/drift_guide.rst Outdated Show resolved Hide resolved
@nirhutnik nirhutnik merged commit 56111cb into main Jul 26, 2022
@delete-merged-branch delete-merged-branch bot deleted the cramersv_vs_psi_14 branch July 26, 2022 18:57
@noamzbr noamzbr modified the milestones: Darwin, Copernicus Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation modification of the documentation / readme's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants