Header Only #2 #3195

shubham1206agra · 2022-04-27T07:25:52Z

Previous one - #3091
Trying to convert remaining files. Details to be added later.

rcurtin · 2022-04-29T01:46:25Z

Nice, looks great so far! I don't know if you had more files you were planning to adapt too.

shubham1206agra · 2022-04-29T04:54:42Z

Nice, looks great so far! I don't know if you had more files you were planning to adapt too.

Yes, I have some files for adaptation. But I was first fixing builds so that I could see results correctly.

src/mlpack/core/dists/gamma_distribution_impl.hpp

shrit

Good, let me know when you need a full review, there is still a lot of work to do

src/mlpack/core/dists/gamma_distribution_impl.hpp

shubham1206agra · 2022-04-30T13:33:49Z

Good, let me know when you need a full review, there is still a lot of work to do

Can you help in random.hpp one?
I am trying to backport global variables header-only pre C++ 17, but unable to do so until now

shubham1206agra · 2022-04-30T16:58:08Z

Good, let me know when you need a full review, there is still a lot of work to do

Can you help in random.hpp one? I am trying to backport global variables header-only pre C++ 17, but unable to do so until now

Finally, its done. Bit of hacky. Can someone confirm whether I need to change some docs or not?

shubham1206agra · 2022-05-10T16:57:38Z

@shrit Please help me in solving these circular dependencies in util files.

src/mlpack/core/util/log.hpp

rcurtin

Hey @shubham1206agra, thank you for working on this! In the util/ directory, the header files are arranged in a careful way. I would love to help out, but it's hard for me to know where things stand---if you want to bring me up to speed, I can try to help, but I wonder if the easiest thing to do here is to revert the changes in util/, proceed with this PR that makes other parts of the library header-only, and then we can approach util/ independently (hopefully keeping the changes small). Let me know what you think. 👍

tried to make log header only changing init of assert adding comment cause checks not started adding corrections cleanup converted version to header only converted timers to header only converted prefixoutsream to .hpp converted program doc to .hpp removing circular dependency removing another circular dependency trying forward decl trying another fwd decl idk now :( forward decl trying correcting dependencies

This reverts commit 6ce2580.

shubham1206agra · 2022-05-11T05:38:29Z

Hey @shubham1206agra, thank you for working on this! In the util/ directory, the header files are arranged in a careful way. I would love to help out, but it's hard for me to know where things stand---if you want to bring me up to speed, I can try to help, but I wonder if the easiest thing to do here is to revert the changes in util/, proceed with this PR that makes other parts of the library header-only, and then we can approach util/ independently (hopefully keeping the changes small). Let me know what you think. 👍

@rcurtin I have reverted all commits related to util for now.
@shrit You can start detailed review for this now.

shubham1206agra · 2022-05-11T05:51:16Z

Reminder for me: I will check e19c00a after sometime later.

@shrit Can you suggest me other ways to implement this?

rcurtin

Awesome, thank you for taking the time to work on this! I went through all the changes and they look fine, but it looks like something has broken some of the tests. My best guess is that the changes to the random code are the issue here---I left a comment there with some more thoughts. Let me know what you think. 👍

src/mlpack/core/dists/laplace_distribution.hpp

src/mlpack/core/math/columns_to_blocks_impl.hpp

src/mlpack/core/math/random.hpp

src/mlpack/methods/radical/radical_impl.hpp

src/mlpack/methods/sparse_autoencoder/sparse_autoencoder_impl.hpp

Co-authored-by: Ryan Curtin <ryan@ratml.org>

… into header-only

src/mlpack/core/math/random.hpp

Co-authored-by: Ryan Curtin <ryan@ratml.org>

rcurtin

Awesome, thanks for the hard work with this!

shrit

@shubham1206agra Brilliant work and clean code; thank you for this outstanding contribution.
@rcurtin are we ready for a release?

shrit · 2022-06-09T13:38:43Z

src/mlpack/methods/local_coordinate_coding/lcc.hpp

+#include <mlpack/methods/sparse_coding/nothing_initializer.hpp>
+#include <mlpack/methods/sparse_coding/data_dependent_random_initializer.hpp>
+#include <mlpack/methods/sparse_coding/random_initializer.hpp>



Thanks for taking care of this 👍

shrit · 2022-06-09T13:45:43Z

Thanks again, all green 💯 💯 🚀

rcurtin · 2022-06-09T13:48:44Z

@rcurtin are we ready for a release?

Almost! I think we still have one or two things to do until we can remove the libmlpack.so target entirely.

shubham1206agra · 2022-06-09T14:35:50Z

@rcurtin are we ready for a release?

Almost! I think we still have one or two things to do until we can remove the libmlpack.so target entirely.

I think we need to take care of util folder.
Everything else belongs to bindings, as far as I know.

rcurtin · 2022-06-09T14:38:22Z

Right, there will be a little bit of trickiness in the util/ folder. The bindings don't need to be adapted.

shubham1206agra added 2 commits April 27, 2022 12:52

converted diagonal guassian distribution to .hpp

abdb3c6

forgot to add inline :)

e7201e2

shubham1206agra added 4 commits April 29, 2022 11:35

converted discrete distribution to .hpp

36f76a9

converted gamma distribution to .hpp

3d18d41

converted guassian to .hpp

2a451e3

changed laplace distribution to .hpp

623717a

shubham1206agra commented Apr 29, 2022

View reviewed changes

src/mlpack/core/dists/gamma_distribution_impl.hpp Outdated Show resolved Hide resolved

shubham1206agra added 11 commits April 29, 2022 12:41

converted regression distribution to .hpp

8b4e4c3

added small comment

626f034

converted ColumnsToBlocks to .hpp

947c736

experimenting ways to export global variable

1e96d07

forgot to remove extern

1b49b18

correction

af6f110

trying extern

087c58d

adding mlpack export in static var

4ab0f48

trying to fix multiple declaration

a8a4397

idk :(

059c0dc

adding extern again :(

f99fd35

shrit requested changes Apr 30, 2022

View reviewed changes

src/mlpack/core/dists/gamma_distribution_impl.hpp Outdated Show resolved Hide resolved

src/mlpack/core/dists/gamma_distribution_impl.hpp Outdated Show resolved Hide resolved

src/mlpack/core/dists/gamma_distribution_impl.hpp Outdated Show resolved Hide resolved

trying different approach

c5bb70f

shubham1206agra added 6 commits May 1, 2022 12:05

converted cosine tree to .hpp

75d3333

fix missing inline

6a63ba1

converted softmax regression to .hpp

4f19adc

converted randomized svd to .hpp

0bef9de

converted radical to .hpp

a13ab0a

converted sparse coding to .hpp

d8f2ea4

rcurtin reviewed May 11, 2022

View reviewed changes

src/mlpack/core/util/log.hpp Outdated Show resolved Hide resolved

rcurtin reviewed May 11, 2022

View reviewed changes

src/mlpack/core/util/log.hpp Outdated Show resolved Hide resolved

rcurtin reviewed May 11, 2022

View reviewed changes

shubham1206agra force-pushed the header-only branch from 542e38f to 6ce2580 Compare May 11, 2022 05:34

Revert "header only util"

c6ac6af

This reverts commit 6ce2580.

rcurtin reviewed May 15, 2022

View reviewed changes

shubham1206agra and others added 7 commits May 16, 2022 11:30

Apply suggestions from code review

45417a5

Co-authored-by: Ryan Curtin <ryan@ratml.org>

Update src/mlpack/methods/sparse_autoencoder/sparse_autoencoder_impl.hpp

458a18b

Co-authored-by: Ryan Curtin <ryan@ratml.org>

trying to remove static build warnings

a851ea7

Merge branch 'header-only' of https://github.com/shubham1206agra/mlpack…

e4f96ee

… into header-only

changed random code

1256ba4

add documentation.

ac5daca

removing thread_local to test failing tests.

6dba88b

rcurtin reviewed Jun 9, 2022

View reviewed changes

src/mlpack/core/math/random.hpp Outdated Show resolved Hide resolved

rcurtin reviewed Jun 9, 2022

View reviewed changes

src/mlpack/core/math/random.hpp Outdated Show resolved Hide resolved

shubham1206agra and others added 2 commits June 9, 2022 09:16

Apply suggestions from code review

769187e

Co-authored-by: Ryan Curtin <ryan@ratml.org>

added back

9e5d62f

rcurtin approved these changes Jun 9, 2022

View reviewed changes

shrit approved these changes Jun 9, 2022

View reviewed changes

shrit merged commit b7e6bd4 into mlpack:master Jun 9, 2022

shubham1206agra deleted the header-only branch June 13, 2022 15:21

rcurtin mentioned this pull request Jun 21, 2022

Adapt virtually everything else in mlpack to header-only #3233

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Header Only #2 #3195

Header Only #2 #3195

shubham1206agra commented Apr 27, 2022

rcurtin commented Apr 29, 2022

shubham1206agra commented Apr 29, 2022

shrit left a comment

shubham1206agra commented Apr 30, 2022 •

edited

shubham1206agra commented Apr 30, 2022

shubham1206agra commented May 10, 2022

rcurtin left a comment

shubham1206agra commented May 11, 2022

shubham1206agra commented May 11, 2022

rcurtin left a comment

rcurtin left a comment

shrit left a comment

shrit Jun 9, 2022

shrit commented Jun 9, 2022

rcurtin commented Jun 9, 2022

shubham1206agra commented Jun 9, 2022

rcurtin commented Jun 9, 2022

Header Only #2 #3195

Header Only #2 #3195

Conversation

shubham1206agra commented Apr 27, 2022

rcurtin commented Apr 29, 2022

shubham1206agra commented Apr 29, 2022

shrit left a comment

Choose a reason for hiding this comment

shubham1206agra commented Apr 30, 2022 • edited

shubham1206agra commented Apr 30, 2022

shubham1206agra commented May 10, 2022

rcurtin left a comment

Choose a reason for hiding this comment

shubham1206agra commented May 11, 2022

shubham1206agra commented May 11, 2022

rcurtin left a comment

Choose a reason for hiding this comment

rcurtin left a comment

Choose a reason for hiding this comment

shrit left a comment

Choose a reason for hiding this comment

shrit Jun 9, 2022

Choose a reason for hiding this comment

shrit commented Jun 9, 2022

rcurtin commented Jun 9, 2022

shubham1206agra commented Jun 9, 2022

rcurtin commented Jun 9, 2022

shubham1206agra commented Apr 30, 2022 •

edited