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

81 layer factory #127

Merged
merged 15 commits into from
Mar 27, 2020
Merged

81 layer factory #127

merged 15 commits into from
Mar 27, 2020

Conversation

ericspod
Copy link
Member

Fixes #81.

Description

Adds a new layer factory mechanism which is extensible and more generic.

Status

Work in progress

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or new feature that would cause existing functionality to change)
  • New tests added to cover the changes
  • Docstrings/Documentation updated

@ericspod ericspod requested review from atbenmurray and wyli February 28, 2020 14:16
Nic-Ma
Nic-Ma reviewed Feb 29, 2020
View reviewed changes
Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Add several inline comments.
Others look good to me.
And we need a clear document to introduce how to use APIs to develop networks.
Otherwise, other contributors may feel hard to add new networks.
Thanks.

@ericspod
Copy link
Member Author

I've finally had a chance to get back to this with updates and changes to DenseNet.

@ericspod ericspod requested a review from Nic-Ma March 26, 2020 14:01
Nic-Ma
Nic-Ma approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@Nic-Ma Nic-Ma 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.

  1. Suggest listing all the supported layer types in the doc-string of layer factory, otherwise, users need to check the code details for that layer type name(like Pool. ADAPTIVEAVG).
  2. And I created another PR 81 update networks in highlights #217 for this PR to introduce the layer factory in MONAI highlights.
    Could you please help review it?

Thanks.

@wyli wyli merged commit e657db0 into master Mar 27, 2020
@wyli wyli deleted the 81-layer-factory branch May 21, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@atbenmurray atbenmurray Awaiting requested review from atbenmurray

@wyli wyli Awaiting requested review from wyli

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

Successfully merging this pull request may close these issues.

Create layer factory mechanism
3 participants
@ericspod @Nic-Ma @wyli