Skip to content

Cleaning up metis#152

Merged
jrh13 merged 34 commits into
jrh13:masterfrom
dnezam:metis-refactor
Feb 27, 2026
Merged

Cleaning up metis#152
jrh13 merged 34 commits into
jrh13:masterfrom
dnezam:metis-refactor

Conversation

@dnezam
Copy link
Copy Markdown
Contributor

@dnezam dnezam commented Feb 19, 2026

As discussed on Zulip: #general > Cleaning up metis?#general > Cleaning up metis?

The main changes are

  • Applying the bug fixes from the upstream metis repo
  • Inlining various definitions, afterwards simplifying where reasonable
  • Removing duplicate and unused definitions
  • Replacing {foo=foo} pattern matching with {foo}
  • Removing the Order module in favor of OCaml's use of integers
    • Transparency: The patch was generated by Claude. I went through it and it looked fine to me (there was one place that looked sketchy, but once I tried the way I would expect it, I realized that it did the correct thing)
  • Moving around some definitions

dnezam added 30 commits February 8, 2026 15:58
This will use the implementation in lib.ml, which is slightly different. In
particular, it seems that the original implementation in metis.ml would not
terminate for n < 0.
In particular, instead of defining the order type, we use the OCaml convention
of using integers.
I think this patch actually makes the code a bit more robust: orderOfInt (and
thus toCompare by extension) would fail if the compare function returned
something other than -1/0/+1, which Repr.compare doesn't seem to exclude.

The applied patch was generated by Claude Code.
Source: gilith/metis/src/Portable.sig
This should make it easier to tell whether something comes from Useful or not,
as the definitions are defined are quite general.
Hopefully, it also makes future refactors easier that want to move things out of
Useful.
@dnezam
Copy link
Copy Markdown
Contributor Author

dnezam commented Feb 19, 2026

A lazy regression suite set up by Claude Code (setting TEST before calling holtest_parallel if I remember correctly) seems to imply that everything is okay, but it's probably a good idea to run a complete regression run (my laptop would turn into a toaster if I were to run the entirety of holtest_parallel)

@jrh13 jrh13 merged commit d9a37c8 into jrh13:master Feb 27, 2026
@dnezam dnezam deleted the metis-refactor branch February 28, 2026 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants