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

Kgao/federal learning #818

Merged
merged 39 commits into from
Nov 2, 2023
Merged

Kgao/federal learning #818

merged 39 commits into from
Nov 2, 2023

Conversation

kgao
Copy link
Collaborator

@kgao kgao commented Oct 4, 2023

Draft PR, please review the documents first.

@kgao kgao requested a review from kbattocchi October 4, 2023 19:20
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Mostly looks good, I've suggested a few minor changes to the code. You also need to fix the linting and documentation building CI failures. I didn't review the .rst file yet, since I'll wait until the docs are generated and look at the resulting HTML

@kgao kgao force-pushed the kgao/federal-learning branch from 06f6f7b to 6c7e485 Compare October 6, 2023 22:26
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Please clean up the documentation, removing all of the generic content and mainly focusing on the actual classes that perform the work in our library. Explaining that this can be used with Spark is helpful, but should not go into a full intro to Spark (and one benefit of how we've implemented this functionality is that it could also be used with any other distributed computing platform).

Also, please merge the latest changes from main into your branch.

kgao and others added 13 commits October 20, 2023 16:34
Signed-off-by: Keith Battocchi <[email protected]>
* enable nans in W

Signed-off-by: Fabio Vera <[email protected]>

* linting

Signed-off-by: Fabio Vera <[email protected]>

* add tests for ests that handle missing in W

Signed-off-by: Fabio Vera <[email protected]>

* allow missing in X for some ortholearner subclasses

Signed-off-by: Fabio Vera <[email protected]>

* refactor keyword arg to be bool only, add more tests

Signed-off-by: Fabio Vera <[email protected]>

* linting

Signed-off-by: Fabio Vera <[email protected]>

* enable missing for metalearners and orf, fix dowhywrapped discretetreat dmlorf

Signed-off-by: Fabio Vera <[email protected]>

* update arg name to allow_missing, add docstrings

Signed-off-by: Fabio Vera <[email protected]>

* add warning when missing values detected

Signed-off-by: Fabio Vera <[email protected]>

* dummy commit

Signed-off-by: Fabio Vera <[email protected]>

* dummy commit revert

Signed-off-by: Fabio Vera <[email protected]>

---------

Signed-off-by: Fabio Vera <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
kbattocchi and others added 4 commits October 31, 2023 12:56
Signed-off-by: Keith Battocchi <[email protected]>
Added Implementation of ray-based distributed parallelization to crossfit.

---------

Signed-off-by: Vishal Verma <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Co-authored-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
@kbattocchi kbattocchi marked this pull request as ready for review October 31, 2023 17:28
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Now looks good, will merge once all checks pass.

@kbattocchi kbattocchi enabled auto-merge (squash) October 31, 2023 17:33
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
Signed-off-by: Keith Battocchi <[email protected]>
@kbattocchi kbattocchi merged commit f170bab into main Nov 2, 2023
@kbattocchi kbattocchi deleted the kgao/federal-learning branch November 2, 2023 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants