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

Example embedded #230

Merged
merged 7 commits into from
Jul 5, 2024
Merged

Example embedded #230

merged 7 commits into from
Jul 5, 2024

Conversation

shrit
Copy link
Member

@shrit shrit commented Jun 27, 2024

I have added one example for now, and a set of CMakefile that I have used over the years to compile mlpack and integrate it into any application. I have never seen any problem with these and they always worked as they should.

The CMakeLists.txt is a simplified version of mlpack original CMakeList.txt and removed everything that is not relevant for a standing application.

PS: it might look like one directory is a duplicate of the other, but this is intended, the first one is a fully working example with a random forest, and the second one is just a set of CMake files that can be copied or adapted, and they are standalone.

…n example

Signed-off-by: Omar Shrit <omar@avontech.fr>
Signed-off-by: Omar Shrit <omar@avontech.fr>
Signed-off-by: Omar Shrit <omar@avontech.fr>
Copy link

Binder 👈 Launch a binder notebook on branch shrit/examples/example_embedded

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Here are some high-level comments---I haven't gone through at a low level of detail yet.

Is CMake the right way to provide this example? All of the other examples have just a Makefile that's very simple (by design). I get that the functionality here is more complicated; there is the autodownloader and the ability to set flags for a specific board, but it feels different than other examples.

I thought while I was writing this that what the example really is is a CMake template for writing embedded applications. Do you think it makes more sense to simply make it into its own repository? It would be possible to even modify it so the autodownloader downloads mlpack itself too, since it's header-only.

The other big question to me is, how do we point users towards this? Should we add to the mlpack main documentation to add a link somewhere? (If so, where?)

And then the last question, which I think is less important, is---the main.cpp currently is just the random forest quickstart or some other trivial example. Should we work that into a more "typical" embedded application? Maybe you're planning to do that later?

I guess my biggest thought has to do with organization: I just want to make sure that a user coming in with no context is able to find this example, adapt it to their needs, and also not get confused by the other examples or types of examples in the repository. Maybe one idea is to split into three example repositories (simple Makefile applications, embedded applications with CMake infrastructure, notebooks)?

embedded/crosscompile_random_forest/main.cpp Outdated Show resolved Hide resolved
Copy link
Member

@rcurtin rcurtin 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 I understand the objective here a little bit better now after a discussion and looking more closely. It's definitely a useful and important thing to add. Some points that we talked about on the call (just for the sake of recording them somewhere):

  • It probably would be a good idea to rename mlpack_embedded_cmake or something like cmake_template or similar, to indicate that it's a template that can be used to start an embedded application.

  • Pointing people towards this example in the right way is probably the most important part here; we talked about an idea of updating doc/index.html or even adding an examples.md page or something like this that points to individual examples (including this one) that is directly accessible from the homepage. Definitely that is not something to do here, just something to keep in mind in the background as we move forward.

  • I also think we may have to do some later restructuring of this repository to handle the three different "forms" of examples we have, but, I don't think either of us have a great idea for how that should look now. As time goes on a better way will probably become clear 😄

embedded/mlpack_embedded_cmake/README.md Outdated Show resolved Hide resolved
embedded/mlpack_embedded_cmake/README.md Outdated Show resolved Hide resolved
embedded/mlpack_embedded_cmake/README.md Outdated Show resolved Hide resolved
embedded/mlpack_embedded_cmake/README.md Outdated Show resolved Hide resolved
embedded/crosscompile_random_forest/main.cpp Show resolved Hide resolved
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. 👍

Signed-off-by: Omar Shrit <omar@avontech.fr>
@shrit
Copy link
Member Author

shrit commented Jul 3, 2024

I will need to push a couple of things to this PR and then we can merge it

shrit added 3 commits July 4, 2024 15:08
Signed-off-by: Omar Shrit <omar@avontech.fr>
Signed-off-by: Omar Shrit <omar@avontech.fr>
Signed-off-by: Omar Shrit <omar@avontech.fr>
@shrit shrit merged commit 4d413b5 into mlpack:master Jul 5, 2024
4 checks passed
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

2 participants