Adding the example for the sensor based data collection and running on a riscv device

[shrit]

No description provided.

[github-actions]

👈 Launch a binder notebook on branch shrit/examples/mpu

[rcurtin]

I am really hesitant about this design. The amount of complexity in this code is huge: it contains its own I2C library specific to each of the sensors in the example, and I think if I understand right that it is basically written entirely by Claude?

(It also seems like the training and inference programs aren't here, but I assume you will push them soon.)

The question I have is: who is the target audience here? Nobody wants to read 3000 lines of hardware control code to understand it. How will someone approach the example? The other examples we have are all written so that someone can read through the entire source and know what is going on. But that's unrealistic here. Do you think we can go through and pare back the code a huge amount? It's an example, not an enterprise application, so all of the "fancy" stuff like short-option and long-option parsing are just unnecessary, in my opinion.

There is a huge amount of other error checking and support code that's also probably unnecessary (at least that is what I have seen with lots of other Claude-generated code). When there is too much code, even if it really is checking for every possible case, a reader will never make it through the whole file and learn what we want them to learn (i.e. how to use mlpack). In my eyes that makes it sort of defeat the purpose of the tutorial.

[rcurtin]

How will we keep this file up to date? There are now copies of mlpack.cmake floating all over the place, but the upstream one in mlpack updates from time to time.

[shrit]

This is a very good question, that I do not have the answer for it, maybe using the CI ? github actions that can check every new PR ?

[rcurtin]

I'm not going to lie: I can't effectively review this PR. Everywhere I look things are either over-complicated by Claude (I will leave a note in the patch application CMake code that it wrote, to show what I mean, but that's it), and the comments are unhelpful.

What is a good comment? I want to be specific about this because it is really hard to quantify it, and at first glance an AI-generated comment like this looks good, but this is entirely the problem with all AI writing: it looks good if you quickly skim it, but if you actually try to take the time to understand it, it is a disaster, because there is no intent behind the words: they are just words. Often, it's way more words than what is needed to actually get the underlying idea across.

A good comment is written specifically to its target audience and assumes some consistent context that a reader is going to have. New concepts are not introduced if they are not necessary, and if they are, then they are explained. So digging into this one:

• "effectively single-core" raises more questions than it answers;
• "OpenMP buys nothing" is really grating; Claude really likes to use metaphors or figures of speech in ways that make its text read like a hustle-porn LinkedIn post, not like something an engineer you trust would have written;
• "its runtime spawns spinning worker threads that starve the program at startup" is just too much information, and I'm not even 100% sure that's a faithful representation of what's happening;
• "Disable it AFTER ..." is a whole huge amount of caveats for something that really just indicates bad design. Why does mlpack.cmake reset the flag? It shouldn't. That's a bug in mlpack.cmake...

At the end of the day, this comment would have been more than enough:

# The Milk-V is single-core.
set(MLPACK_DISABLE_OPENMP ON)

In any case, this is a massive amount of text and code to ask me to review and I can't give you a reasonable review when so much of it is AI-written. What I would like to ask you to do is go through, make sure you understand every bit of the code, replace the garbage comments with ones that are actually something a human would write (even if you use Claude to fix them grammatically or whatever, that's totally fine). As you do this you will find tons of things that are needlessly overcomplicated and you can simplify them (or even have Claude help with that, although personally I find it a lot more edifying to just do it myself, especially when it's simple). At the end of the process, this whole thing will be a lot easier to review and also, very importantly, a lot easier for someone else to come along and understand.

Here are two things I read recently that I highly recommend that are relevant to what I am trying to get at about AI writing (both came across HN, which, honestly, I would like to just stop reading, but, occasionally I find good things there):

• https://cymerys.com/w/im-becoming-ai-blind
• https://www.experimental-history.com/p/i-like-em-thick

[rcurtin]

This is an insanely overcomplicated patch applier that is basically impossible to quickly understand because it tries to do so much error handling. It would be better without any of the error handling. I wrote something that did this in the Milk-V CI PR:

https://github.com/mlpack/mlpack/pull/4234/changes#diff-3e58bfa6d751b0ea06062ba1f3becd2b8d27ac6df3a820b3ec3516802c04bad7

Take a look at line 657 onwards of mlpack.cmake.

Every little bit of extra error handling and extra complexity is us accepting responsibility for a user problem. Here, we try to find whether the patch will apply cleanly or not and skip it if it's already been applied. That sounds nice, but why? We should only be compiling OpenBLAS once, at the moment of download, and if the patch does not apply cleanly then, then something is clearly wrong and we should throw a failure instead of ignoring it.

What is the cost of this extra code? It might look like nothing since it took Claude five seconds, but, when we accept the responsibility for the user problem (patch does not apply cleanly because something external to our code changed), we accept the responsibility for maintaining this code when something goes wrong, and that takes way way more than five seconds.

You might think, "but this code looks good! The likelihood of it failing in the future is low because it will not be changed!" But somewhere in all the changes for applying patches to OpenBLAS, there is a bug. I'll leave it up to you to find it... see how long it takes to decide what the right fix is (since Claude might be able to find the bug I am talking about quickly... but it also might not, there is no guarantee...), then decide whether it is really worth it to let Claude blast out code like this. :)

[rcurtin]

We should definitely not have RISC-V-specific code in this file at all. It should go entirely into crosscompile-arch-config.cmake, so that everything related to a particular architecture is encapsulated in only one place. In the PR I opened here, I did that approach instead.

If you really think this approach is better, and there is something I overlooked, I'm totally open to a discussion about it, but, I am pretty sure this was something Claude chose and I disagree pretty strongly with it.