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.

Sign up for GitHub

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

Jump to bottom

lfr cleanup and update. #187

Merged
merged 9 commits into from
Jun 29, 2020
Merged

lfr cleanup and update. #187

merged 9 commits into from
Jun 29, 2020

Conversation

pronics2004
Copy link
Contributor

@pronics2004 pronics2004 commented Jun 22, 2020 •
edited
Loading

Removed numba dependency and cleaned up the code for consistent terminology.

This implementation is closer to vanilla LFR, where the feature distance computation is standard l2-distance without any learned scaling parameters (alpha).

@pronics2004 pronics2004 reopened this Jun 23, 2020
hoffmansc
hoffmansc requested changes Jun 25, 2020
View reviewed changes
aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr_helpers/helpers.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr_helpers/helpers.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr_helpers/helpers.py Outdated Show resolved Hide resolved
nrkarthikeyan
nrkarthikeyan approved these changes Jun 26, 2020
View reviewed changes
Copy link
Collaborator

@nrkarthikeyan nrkarthikeyan left a comment

Choose a reason for hiding this comment

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

Looks good to me Prasanna. I did not look at the technicalities too much, but looks good to me.

aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr_helpers/helpers.py Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr.py Show resolved Hide resolved
examples/demo_lfr.ipynb Show resolved Hide resolved
hoffmansc
hoffmansc requested changes Jun 26, 2020
View reviewed changes
Copy link
Collaborator

@hoffmansc hoffmansc left a comment

Choose a reason for hiding this comment

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

We can also get rid of the except part of the import in lfr.py (lines 7-10).

Looks good otherwise!

aif360/algorithms/preprocessing/lfr.py Outdated Show resolved Hide resolved
aif360/algorithms/preprocessing/lfr_helpers/helpers.py Outdated Show resolved Hide resolved
@hoffmansc
Copy link
Collaborator

We can also get rid of the except part of the import in lfr.py (lines 7-10).

@pronics2004 other than this, looks great. Thanks!

@pronics2004
Copy link
Contributor Author

We can also get rid of the except part of the import in lfr.py (lines 7-10).

@pronics2004 other than this, looks great. Thanks!

Done. Thanks!

@pronics2004 pronics2004 self-assigned this Jun 29, 2020
@pronics2004 pronics2004 merged commit c6dfe61 into master Jun 29, 2020
Illia-Kryvoviaz pushed a commit to Illia-Kryvoviaz/AIF360 that referenced this pull request Jun 7, 2023
@pronics2004
Merge pull request Trusted-AI#187 from IBM/lfr-update
11d5c49 
lfr cleanup and update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nrkarthikeyan nrkarthikeyan nrkarthikeyan approved these changes

@hoffmansc hoffmansc hoffmansc approved these changes

Assignees

@pronics2004 pronics2004

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pronics2004 @hoffmansc @nrkarthikeyan