-
Notifications
You must be signed in to change notification settings - Fork 4
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
add setup.py, workflows, unit tests, and examples #1
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: kta-intel <[email protected]>
… some basic tests Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
Signed-off-by: kta-intel <[email protected]>
…example Signed-off-by: kta-intel <[email protected]>
… and workflows Signed-off-by: kta-intel <[email protected]>
…e test Signed-off-by: kta-intel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting PR no. 1, @kta-intel 😉 ! It's great that you have created some basic infrastructure and examples for contributors to start building and testing with.
I have a couple of suggestions for making the repo structure a bit more intuitive - but otherwise I believe it's in pretty good shape already!
|
||
setup( | ||
name='openfl-contrib', | ||
version='1.6', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openfl-contrib
being a newly minted repo, shouldn't we start from 0.1.0
as per semantic versioning?
@@ -0,0 +1,3 @@ | |||
# Copyright (C) 2020-2023 Intel Corporation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - 2024 (noticed a couple of similar occurrences across the PR)
# Copyright (C) 2020-2024 Intel Corporation | ||
# SPDX-License-Identifier: Apache-2.0 | ||
"""openfl-contrib version information.""" | ||
__version__ = '1.6' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here - 0.1.0
maybe for openfl-contrib
specifically?
# Copyright (C) 2020-2023 Intel Corporation | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
"""Aggregation functions package.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to keep the aggregation_functions
package right under openfl_contrib
. I think the intermediate interface
package is a bit confusing. Essentially, the openfl_contrib package could look like this, additionally highlighting the types of contrib-s (pun intended 😅) suitable for this repo:
└── openfl-contrib
└── aggregation_functions
└── (compression_)pipelines
└── ...
|
||
__all__ = [ | ||
'CustomWeightedAverage' | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - the indentation looks a bit off here
@@ -0,0 +1,3 @@ | |||
# Copyright (C) 2020-2023 Intel Corporation | |||
# SPDX-License-Identifier: Apache-2.0 | |||
"""Tests package.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember seeing a similar package structure somewhere under openfl
, but I don't understand very well why we need a separate github
package (supposedly for tests running in the CI?). I suggest to simplify, and move the the test_task_runner.py
module right under tests/openfl_contrib
.
This PR adds:
setup.py
to enable installation ofopenfl-contrib
extension on top of latest baseopenfl
openfl_contrib
folder structure to mirroropenfl
, containing:openfl_contrib.interface.aggregation_functions
with a custom weighted aggregation function (custom implementation of OpenFL's weighted average)openfl_contrib.pipelines
with an SKC compression pipeline (borrowed directly from base OpenFL)openfl_contrib_workspace
to demonstrate how to incorporate custom plan with new agg functions and compression pipelines. Will also serve as the workspace for Task Runner test.github/workflows
to run lint and teststests
directory containing:github
for CI testing of Task Runner API with custom plan andCustomWeightedAverage
andSKCPipeline
openfl-contrib
for individual unit tests forCustomWeightedAverage
andSKCPipeline