Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

XP-357 make controller parametrisable #201

Merged
merged 4 commits into from
Jan 8, 2020
Merged

XP-357 make controller parametrisable #201

merged 4 commits into from
Jan 8, 2020

Conversation

wilk10
Copy link
Contributor

@wilk10 wilk10 commented Jan 3, 2020 •
edited by janpetschexain
Loading

References

XP-357

Summary

Corrects the initial implementation of Participant selection by initialising the Controller only once and then passing the participant_ids only when calling select_ids on the controller.

Are there any open tasks/blockers for the ticket?

None

Reviewer checklist

Reviewer agreement:

  • Reviewers assign themselves at the start of the review.
  • Reviewers do not commit or merge the merge request.
  • Reviewers have to check and mark items in the checklist.

Merge request checklist

  • Conforms to the merge request title naming XP-XXX <a description in imperative form>.
  • Each commit conforms to the naming convention XP-XXX <a description in imperative form>.
  • Linked the ticket in the merge request title or the references section.
  • Added an informative merge request summary.

Code checklist

  • Conforms to the branch naming XP-XXX-<a_small_stub>.
  • Passed scope checks.
  • Added or updated tests if needed.
  • Added or updated code documentation if needed.
  • Conforms to Google docstring style.
  • Conforms to XAIN structlog style.

janpetschexain
janpetschexain suggested changes Jan 6, 2020
View reviewed changes
xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Show resolved Hide resolved
@wilk10 wilk10 force-pushed the XP-357_make_controller_parametrisable branch from 249d0e4 to b974f44 Compare January 7, 2020 10:00
janpetschexain
janpetschexain previously approved these changes Jan 7, 2020
View reviewed changes
finiteprods
finiteprods suggested changes Jan 7, 2020
View reviewed changes
Copy link
Contributor

@finiteprods finiteprods 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, just a few very tiny typos I spotted.

xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
xain_fl/fl/coordinator/controller.py Outdated Show resolved Hide resolved
@wilk10 wilk10 force-pushed the XP-357_make_controller_parametrisable branch from 75acace to 1f7e4ef Compare January 8, 2020 08:24
Robert-Steiner
Robert-Steiner previously approved these changes Jan 8, 2020
View reviewed changes
finiteprods
finiteprods previously approved these changes Jan 8, 2020
View reviewed changes
@wilk10 wilk10 force-pushed the XP-357_make_controller_parametrisable branch from 1f7e4ef to b0c2869 Compare January 8, 2020 15:02
@wilk10 wilk10 merged commit ecf53ea into development Jan 8, 2020
@wilk10 wilk10 deleted the XP-357_make_controller_parametrisable branch January 8, 2020 15:12
rsaffi pushed a commit that referenced this pull request Jan 23, 2020
@atymoshchuk @dkravetz
@PanicButtonPressed @wilk10 @Robert-Steiner @little-dude @janpetschexain @finiteprods
XP-227 - Merge v0.3.0 changes into master (#233)
fed62b7 
* XP-337 Clean up docs before generation (#188)

* XP-229 Update Readme.md (#189)

* XP-255 update codeowners and authors in setup (#195)

* XP-265 move benchmarks to separate repo (#193)

* XP-243 remove flags from xain-fl codebase

* XP-255 update codeowners and authors in setup (#195)

* XP-257 cleanup cproto dir (#198)

* XP-261 move tests to own dir (#197)

* XP-168 update setup.py (#191)

* DO-35 📰 ✨ Add placeholder for e2e. Update CI for gitflow

* XP-241 remove legacy participant and sdk dir (#199)

* DO-17 🐳 ✨ Add Dockerfiles, dockerignore and docs (#202)

* XP-354 Remove proto files (#200)

* XP-385 Fix docs badge (#204)

* Xp 273 scripts cleanup (#206)

* XP-357 make controller parametrisable (#201)

* XP-384 remove unused files (#209)

* Do 43 docker compose minio (#208)

* XP-374 Clean up docs (#211)

* XP-271 fix pylint issues (#210)

* XP-424 Remove unused packages (#212)

* DO-49 🐳 ✨ Create initial buckets (#213)

* Xp 373 add sdk as dependency in fl (#214)

* XP-433 Fix docker headings (#218)

* XP-119 Fix gRPC testing setup so that it can run on macOS (#217)

* XP-422 ai metrics (#216)

* XP-308: store aggregated weights in S3 buckets (#215)

* XP-436 Reinstate FINISHED heartbeat from Coordinator (#219)

* XP-208 model sum aggregator as a mock for testing

* XP-208 add test integrating P and C

* XP-208 add identity controller as a mock for test

* XP-208 small typo in id-controller docstring

* XP-208 add coordinator test fixture with mocked components

* XP-208 test for top-level function start_participant
* XP-308: print debugging information for spurious failure

* XP-208 join monitor_thread to confirm response to terminate_event

* XP-208 change mock patch string to object called rather than defined

* XP-208 model sum aggregator as a mock for testing

* XP-208 add test integrating P and C

* XP-208 add identity controller as a mock for test

* XP-208 small typo in id-controller docstring

* XP-208 add coordinator test fixture with mocked components

* XP-208 test for top-level function start_participant

* XP-436 fix bug: coordinator not advertising FINISHED state

* XP-436 revert logging level

* XP-208 join monitor_thread to confirm response to terminate_event

* XP-208 change mock patch string to object called rather than defined

* XP-436 suggested documentation fixes from review

* XP-208 mock the store (following rebase)

* XP-208 fix import

* XP-208 test_start_participant: mark as slow, comment on aggregation

* XP-480 revise message names (#222)

* XP-499 Remove conftest, exclude tests folder (#223)

* XP-333 Replace numproto with xain-proto (#220)

* XP-505: docstrings cleanup (#224)

* XP-508 Replace circleci badge (#225)

* XP-510 allow for zero epochs on cli (#227)

* XP-498: more generic shebangs (#229)

* XP-505: cleanup docstrings in xain_fl.coordinator (#228)

* Prepare release of v0.3.0 (matching version with `xain-sdk` and `xain-proto`) (#230)

Co-authored-by: Anastasiia Tymoshchuk <atymoshchuk@icloud.com>
Co-authored-by: Daniel Kravetz <daniel.kravetz@gmail.com>
Co-authored-by: Felix Reichel <31513604+PanicButtonPressed@users.noreply.github.com>
Co-authored-by: Anselmo Sampietro <ans.samp@gmail.com>
Co-authored-by: Robert Steiner <robertt.debug@gmail.com>
Co-authored-by: Corentin Henry <little-dude@users.noreply.github.com>
Co-authored-by: janpetschexain <58227040+janpetschexain@users.noreply.github.com>
Co-authored-by: kwok <kwok.doc@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@PanicButtonPressed PanicButtonPressed PanicButtonPressed approved these changes

@finiteprods finiteprods finiteprods approved these changes

@janpetschexain janpetschexain janpetschexain left review comments

@Robert-Steiner Robert-Steiner Robert-Steiner left review comments

@atymoshchuk atymoshchuk Awaiting requested review from atymoshchuk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@wilk10 @finiteprods @Robert-Steiner @PanicButtonPressed @janpetschexain