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

Implementing Simple Radial Basis Function Layer #2261

Merged
merged 23 commits into from Jun 16, 2020

Conversation

geekypathak21
Copy link
Member

@geekypathak21 geekypathak21 commented Mar 6, 2020

Hey @zoq I am trying to add simple RBF network in mlpack. Task needed to be completed are as follows. It's not completed yet I will try to complete it. Please suggest if you have any inputs for this.

  • Add Documentation for RBF
  • Add Implementation of RBF
  • Add Some Activation Functions
  • Tests for new Neural Network

@rcurtin
Copy link
Member

rcurtin commented Mar 12, 2020

This code touches the ANN code, and there was recently a big refactoring of the ANN code in #2259, so be sure to merge the master branch into your branch here to make sure that nothing will fail if this PR is merged. 👍 (I'm pasting this message into all possibly relevant PRs.)

Copy link
Member

@favre49 favre49 left a comment

Choose a reason for hiding this comment

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

There are a lot of style issues, please use the style checks and fix them all

src/mlpack/methods/ann/layer/radial_basis_function.hpp Outdated Show resolved Hide resolved
src/mlpack/methods/ann/layer/radial_basis_function.hpp Outdated Show resolved Hide resolved
src/mlpack/methods/ann/layer/radial_basis_function.hpp Outdated Show resolved Hide resolved
src/mlpack/methods/ann/layer/base_layer.hpp Show resolved Hide resolved
@mlpack-bot
Copy link

mlpack-bot bot commented Apr 17, 2020

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

@mlpack-bot mlpack-bot bot added the s: stale label Apr 17, 2020
@geekypathak21
Copy link
Member Author

keep open

@geekypathak21 geekypathak21 force-pushed the add-rbf branch 2 times, most recently from c6a8f87 to 4ed29cf Compare May 14, 2020 19:57
@saksham189
Copy link
Member

Also could you give me a link/reference to an implementation which you are referring to implement this. In most implementation I see the centres and scaling factors are being trained as well.

@saksham189
Copy link
Member

Alright, so I do not think that we need the Gradient method. Since mlpack already has an implementation of kmeans I think it would be fine to pass the centres through the constructor.

@geekypathak21
Copy link
Member Author

I will try and get back to you @saksham189

@saksham189
Copy link
Member

saksham189 commented Jun 12, 2020

Hey @himanshupathak21061998 I think the work here is almost complete.
Can you see why the static code analysis check is failing and maybe clean up the commit history?

typename Activation>
RBF<InputDataType, OutputDataType, Activation>::RBF() :
inSize(0),
outSize(0)
Copy link
Member

Choose a reason for hiding this comment

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

can you initialise betas and sigmas to 0 here. I think that should fix the static code analysis check.

@geekypathak21 geekypathak21 force-pushed the add-rbf branch 2 times, most recently from 47b544c to 151fc51 Compare June 12, 2020 19:52
@saksham189
Copy link
Member

Hey Himanshu did you try running the test locally with random seed?

@geekypathak21
Copy link
Member Author

Yup, @saksham189 already did see the comment above

I have tested with random seed locally.
When without setting beta parameter

classificationError = 0.174   for centroids = 10
classificationError = 0.13   for centroids = 20
classificationError = 0.092   for centroids = 30
classificationError = 0.092   for centroids = 40
classificationError = 0.092   for centroids = 50
classificationError = 0.078   for centroids = 60
classificationError = 0.068   for centroids = 70
classificationError = 0.088   for centroids = 80
classificationError = 0.072   for centroids = 90
classificationError = 0.07   for centroids = 100
classificationError = 0.074   for centroids = 110
classificationError = 0.09   for centroids = 120
classificationError = 0.084   for centroids = 130
classificationError = 0.06   for centroids = 140
classificationError = 0.094   for centroids = 150

When setting betas=4.1

classificationError = 0.192   for centroids = 10
classificationError = 0.124   for centroids = 20
classificationError = 0.07   for centroids = 30
classificationError = 0.082   for centroids = 40
classificationError = 0.032   for centroids = 50
classificationError = 0.068   for centroids = 60
classificationError = 0.038   for centroids = 70
classificationError = 0.042   for centroids = 80
classificationError = 0.046   for centroids = 90
classificationError = 0.02   for centroids = 100
classificationError = 0.03   for centroids = 110
classificationError = 0.026   for centroids = 120
classificationError = 0.032   for centroids = 130
classificationError = 0.02   for centroids = 140
classificationError = 0.026   for centroids = 150

@saksham189
Copy link
Member

saksham189 commented Jun 13, 2020

alright great! can you run the current test locally multiple times and see if it continues to pass?
(just to make sure)

@geekypathak21
Copy link
Member Author

alright great! can you run the current test locally multiple times and see if it continues to pass?
(just to make sure)

Sorry for the late reply actually I have to rebuild the project which took some time I ran the tests 20 times and with no failure. I will start working on SVM from tommorow.

Copy link
Member

@saksham189 saksham189 left a comment

Choose a reason for hiding this comment

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

I think this looks good to merge if you just make the 2 minor fixes :)

src/mlpack/tests/feedforward_network_test.cpp Outdated Show resolved Hide resolved
Co-authored-by: saksham189 <7020962+saksham189@users.noreply.github.com>
Copy link
Member

@saksham189 saksham189 left a comment

Choose a reason for hiding this comment

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

Thanks for the work on this!

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@saksham189 saksham189 merged commit 398a276 into mlpack:master Jun 16, 2020
@saksham189
Copy link
Member

Thanks again @himanshupathak21061998 !

@geekypathak21
Copy link
Member Author

Thanks for merging @saksham189

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.

None yet

6 participants