Recompute flag in clojure math runner is ignored, and somewhat unclear in config

[jucor]

TLDR: @metasoarous , do you remember please:

• did you you ever managed to get the recompute flag working as you wanted?
• if so, what cases did you use it for, please?
• or is it vestigial code that could be removed? Thanks !

Details:

The clojure runner for math worker has a --recompute flags it advertises, which is supposed to "recompute conversations from scratch", rather than incrementally from last known position: https://github.com/compdemocracy/polis/blob/52652e443c52bb554536ef029ce4c951826039c3/math/src/polismath/runner.clj#L82-L84

I was hoping to use it for several tests.

However, it seems that while it is parsed, it is not used. If I understand correctly the clojure code-base (with Cursor's help, so I'm treading carefully), it should be passed to conv-man/load-or-init, but it is not: https://github.com/compdemocracy/polis/blob/52652e443c52bb554536ef029ce4c951826039c3/math/src/polismath/runner.clj#L101-L104

That function itself would then (were it called with that keyword, which it isn't) take it into account here: https://github.com/compdemocracy/polis/blob/52652e443c52bb554536ef029ce4c951826039c3/math/src/polismath/conv_man.clj#L188-L207 but the surprise expressed in the comments make me think it might not be entirely working as intended...

Beyond the runner, the load-or-init is only called in one place with potentially the :recompute flag, in conv-actor: https://github.com/compdemocracy/polis/blob/52652e443c52bb554536ef029ce4c951826039c3/math/src/polismath/conv_man.clj#L375-L378 depending on the configuration defined in components/config.clj, which also come with comments showing some amount of uncertainty: https://github.com/compdemocracy/polis/blob/52652e443c52bb554536ef029ce4c951826039c3/math/src/polismath/components/config.clj#L107-L109

This would require the RECOMPUTE environment variable to set that flag. It is not set in example.env, nor can I see any trace of it anywhere else in the codebase. Therefore if it has ever been used, it has been done manually on an instance.

So it's unclear how much the recompute feature has been used, and for what, and whether the runner should be modified to allow for its use, or on the contrary whether the recompute functionality is vestigial code that could be removed.

[jucor]

For context: I was (thinking I was) using this functionality when tracing computations as part of #1893, to make sure I was using only the voting matrix regardless of whatever previous state the clojure math worker may have previously stored in various SQL blobs (I haven't gotten around to understanding those blobs yet, hence wanted isolation from them).

[metasoarous]

I do write the best code comments, don't I?

Or did...

Anyway, this setting was something we used for fully recomputing conversation state from scratch. Looking back, I think I know what it's doing there, but my comments do collectively evidence that this setting was somewhat confusingly implemented. The main thing is that the config setting is just a boolean flag, but there are actually different levels of "recompute". The :recompute reboot (IIRC) effectively just rehydrates the conversation state from the math blob export, so that's the right thing to do if (not recompute). Meanwhile, :recompute :full starts the conversation from scratch, ignoring previous PCA loadings and K-means cluster centroids, and that method gets used when either the --recompute flag or RECOMPUTE env var are set.

This wasn't something we frequently used, but was implemented after we found an issue with the math results on certain conversations, and gave us a way to "reset" the results from scratch, to therefore avoid the iterative updates continually trying to build on an effectively broken conversation state. Whether it's still necessary is up for debate. If we're keeping iterative updates, it probably makes sense to keep as a safeguard, but ideally there would be a way to only run it on a select number of conversations.

An alternative might be to just design the implementation so that nuking the old math blob triggers a full recompute. That may be easier to trigger on a case by case basis, but would still require a process of stop math worker -> remove blob -> restart math worker.

[jucor]

You absolutely do, they're really fun to read, and show the passion that went into the code! It's a treat :)

Thanks for all the info and the quick reply! I'll digest it as I go through the full logic. Digesting 13 years of work is an endeavour 😅