Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • Fed-BioMed Fed-BioMed
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 130
    • Issues 130
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 3
    • Merge requests 3
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Terraform modules
  • Monitor
    • Monitor
    • Metrics
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Fed-BioMedFed-BioMed
  • Fed-BioMedFed-BioMed
  • Merge requests
  • !188

Feature/472 create model abstraction for declearn integration

  • Review changes

  • Download
  • Patches
  • Plain diff
Merged BOUILLARD Yannick requested to merge feature/472-create-model-abstraction-for-declearn-integration into develop Feb 17, 2023
  • Overview 99
  • Commits 56
  • Pipelines 14
  • Changes 19

MR description

Implements model abstraction for pytorch and scikit learn Closes #472 (closed).

Here a list of points that i found difficult to implement (and that need a closer look).

  1. BaseSklearnModel:
  • batch_size computation / reset that is done internally
  • sklearn model n_iter attribute increment/decrement (hidden through methods)
  • apply_updates method: slightly modified from the one of the poc, that adds gradients instead of changing them, in the same spirit as the apply_updates of pytorch. Check if computation is correct
  • Toolbox classes, that implement some method using multiple inheritance
  1. TorchModel
  • method train not implemented: should we compute loss in this method ?
  • is get_gradients method correct?
  • saving state of the TorchModel: initial parameters are saved in a specific attribute:
  • make sure it handles frozen layers

More broadly speaking:

  • appropriate raise of exceptions: some exceptions may not be caught
  • appropriate naming of variables
  • create multiple modules for each classes (ie one class per file) rather than having 3 classes in a file.

Developer Certificate Of Origin (DCO)

By opening this merge request, you agree the Developer Certificate of Origin (DCO)

This DCO essentially means that:

  • you offer the changes under the same license agreement as the project, and
  • you have the right to do that,
  • you did not steal somebody else’s work.

License

Project code files should begin with these comment lines to help trace their origin:

# This file is originally part of Fed-BioMed
# SPDX-License-Identifier: Apache-2.0

Code files can be reused from another project with a compatible non-contaminating license. They shall retain the original license and copyright mentions. The CREDIT.md file and credit/ directory shall be completed and updated accordingly.

Guidelines for MR review

General:

  • give a glance to DoD
  • check coding rules and coding style
  • check docstrings (eg run tests/docstrings/check_docstrings)

Specific to some cases:

  • update all conda envs consistently (development and vpn, Linux and MacOS)
  • if modified researcher (eg new attributes in classes) check if breakpoint needs update (breakpoint/load_breakpoint in Experiment(), save_state/load_state in aggregators, strategies, secagg, etc.)
Edited Feb 22, 2023 by BOUILLARD Yannick
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: feature/472-create-model-abstraction-for-declearn-integration