Added accessor methods for ANN layers. #2321
Conversation
|
Can you merge the current master branch? |
Update PR with latest changes made upstream. Date : 25 March
|
Done! |
|
This PR is marked as a draft, but I think it's ready? |
|
The code is pretty much ready. I just kept it as a draft because I haven't written any test cases yet for the functions I added. Anyway, I am marking it as ready now. Let me know if there are any changes to make. |
|
Thanks for the clarification, adding tests would be great! |
|
I will get down to writing the tests as soon as I can, maybe in a day or two. And then maybe, you can review it? Thanks! |
|
Sounds good, no hurry. |
iamshnoo
force-pushed
the
accessor_methods_ann
branch
from
87c2701 to
9672d5d
Compare
|
I had to close and re-open this because of a tiny error I made when doing a git reset. |
kartikdutt18
left a comment
kartikdutt18
left a comment
There was a problem hiding this comment.
Hey @iamshnoo, This looks good to me. Thanks a lot for working on this. I have left some very minor comments. Other than that this is great. Could you also fix the merge conflict, Thanks.
Thanks a lot for the contribution.
|
Just resolved the merge conflict, so make sure to run |
Co-authored-by: kartikdutt18 <[email protected]>
kartikdutt18
left a comment
kartikdutt18
left a comment
There was a problem hiding this comment.
This looks good to me. Thanks a lot for working on this. 👍
zoq
left a comment
zoq
left a comment
There was a problem hiding this comment.
Agreed, this looks really good, can you update HISTORY.md?
zoq
left a comment
zoq
left a comment
There was a problem hiding this comment.
Thanks for keeping up with the comments, no further comments from my side.
|
Hello there! Thanks for your contribution. I see that this is your first contribution to mlpack. If you'd like to add your name to the list of contributors in In addition, if you'd like some stickers to put on your laptop, I'd be happy to help get them in the mail for you. Just send an email with your physical mailing address to [email protected], and then one of the mlpack maintainers will put some stickers in an envelope for you. It may take a few weeks to get them, depending on your location. 👍 |
|
Thanks again for the contribution. |
5 participants
This PR adds accessor methods for different layer parameters. The tests are present in the ann_layer_tests.cpp file and all of them have a similarity in their names (each one ends with LayerParametersTest).
Note : After messing up my PR in #2307, I deleted my fork and re-did everything in this new forked branch. All the changes are still the same. And I am yet to complete writing tests for this.
This solves Issue #2258 temporarily to some extent for now.