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

Expose the instantiated network. #2043

Merged
merged 3 commits into from Oct 5, 2019

Conversation

@zoq
Copy link
Member

zoq commented Oct 3, 2019

Expose the the instantiated network; see #2041 for more details.

zoq added 2 commits Oct 3, 2019
Copy link
Member

ShikharJ left a comment

I like the changes, they're minimal.

@rcurtin
rcurtin approved these changes Oct 4, 2019
Copy link
Member

rcurtin left a comment

Looks good. This is maybe not in scope for this PR, but do you think that we could also expose the individual weights for each layer via something like this?

arma::mat LayerParameters(const size_t layerIndex)
{
  // I'm not sure, but maybe is it possible to use a slightly different visitor and return an `arma::mat&`?
  arma::mat result;
  boost::apply_visitor(ParametersVisitor(std::move(result)), model[layerIndex]);
}

I'm not sure that's the best writeup of the idea there, but maybe it's a start at an idea. Anyway, just a thought, maybe useful, maybe not. :)

@@ -265,7 +273,7 @@ class FFN
//! Modify the initial point for the optimization.
arma::mat& Parameters() { return parameter; }

//! Get the matrix of responses to the input data points.
//! Get the matrix of resposnses to the input data points.

This comment has been minimized.

Copy link
@rcurtin

rcurtin Oct 4, 2019

Member

Oops, looks like a typo got introduced.

This comment has been minimized.

Copy link
@zoq

zoq Oct 4, 2019

Author Member

Opps, fixed in dae940f.

@zoq

This comment has been minimized.

Copy link
Member Author

zoq commented Oct 4, 2019

I like the LayerParameters idea, will open another PR.

@mlpack-bot
mlpack-bot bot approved these changes Oct 5, 2019
Copy link

mlpack-bot bot left a comment

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit dae940f into mlpack:master Oct 5, 2019
6 checks passed
6 checks passed
LaTeX Documentation Checks Build finished.
Details
Memory Checks Build finished.
Details
Static Code Analysis Checks Build finished.
Details
Style Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rcurtin

This comment has been minimized.

Copy link
Member

rcurtin commented Oct 5, 2019

Awesome, happy to have this merged. 👍

@ricardo-rodrigues-carmo

This comment has been minimized.

Copy link

ricardo-rodrigues-carmo commented Oct 7, 2019

Thanks guys 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.