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

[ccapi] Syntactic sugar constructors #735

Merged
merged 2 commits into from Nov 19, 2020

Conversation

kparichay
Copy link
Member

@kparichay kparichay commented Nov 11, 2020

Added syntactic sugar constructors for optimizers
which allow making optimizer closer to existing API
and more readable.

Resolves #734

Self evaluation:

  1. Build test: [x]Passed [ ]Failed [ ]Skipped
  2. Run test: [x]Passed [ ]Failed [ ]Skipped

Signed-off-by: Parichay Kapoor pk.kapoor@samsung.com

@taos-ci
Copy link
Collaborator

taos-ci commented Nov 11, 2020

📝 TAOS-CI Version: 1.4.20191203. Thank you for submitting PR #735. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://nnsuite.mooo.com/.

Copy link
Contributor

@zhoonit zhoonit left a comment

Choose a reason for hiding this comment

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

LGTM

@kparichay
Copy link
Member Author

@zhoonit My apologies, but can you please review once more. Direct methods are added for layers and loss as well.

return layer;
}

namespace loss {
Copy link
Contributor

@zhoonit zhoonit Nov 11, 2020

Choose a reason for hiding this comment

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

Are we gonna open up loss layer? I think our code is not ready for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about we have an alias sth like using Loss = Layer?

Copy link
Member Author

@kparichay kparichay Nov 11, 2020

Choose a reason for hiding this comment

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

The loss_layer was already opened. However, there was no way to choose the loss type for the created loss layer.
This patch allows that.
Handling adding loss layer externally, I am handling that now in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about we have an alias sth like using Loss = Layer?

creating two namespace purpose is to keep loss and layer decoupled. If later, loss is removed a layer, it would require updates here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay

@kparichay kparichay changed the title [ccapi] Syntactic sugar constructors [WIP] [ccapi] Syntactic sugar constructors Nov 11, 2020
@kparichay kparichay changed the title [WIP] [ccapi] Syntactic sugar constructors [ccapi] Syntactic sugar constructors Nov 11, 2020
@zhoonit
Copy link
Contributor

zhoonit commented Nov 12, 2020

As #721 has been merged, please wait for #740

@kparichay kparichay changed the title [ccapi] Syntactic sugar constructors [#740] [ccapi] Syntactic sugar constructors Nov 12, 2020
EXPECT_NO_THROW(ml::train::createLayer(ml::train::LayerType::LAYER_ADDITION));
EXPECT_NO_THROW(ml::train::createLayer(ml::train::LayerType::LAYER_LOSS));
EXPECT_NO_THROW(ml::train::layer::Input());
EXPECT_NO_THROW(ml::train::layer::FullyConnected());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good.

Copy link
Collaborator

@jijoongmoon jijoongmoon 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.

Added syntactic sugar constructors for optimizers
which allow making optimizer closer to existing API
and more readable.

See also nnstreamer#734

**Self evaluation:**
1. Build test: [x]Passed [ ]Failed [ ]Skipped
2. Run test: [x]Passed [ ]Failed [ ]Skipped

Signed-off-by: Parichay Kapoor <pk.kapoor@samsung.com>
Added syntactic sugar constructors for layers and loss
This allows creating various layers and losses directly with c++ API

**Self evaluation:**
1. Build test: [x]Passed [ ]Failed [ ]Skipped
2. Run test: [x]Passed [ ]Failed [ ]Skipped

Signed-off-by: Parichay Kapoor <pk.kapoor@samsung.com>
@kparichay kparichay changed the title [#740] [ccapi] Syntactic sugar constructors [ccapi] Syntactic sugar constructors Nov 17, 2020
@jijoongmoon jijoongmoon merged commit 8d43377 into nnstreamer:main Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ccapi add syntactic sugar
4 participants