Add (new) mpl benchmarks

[Firobe]

Close #401

Add msort_ints, msort_strings, primes, tokens, raytracer from https://github.com/MPLLang/parallel-ml-bench/tree/main/ocaml/bench to the parallel benchs (with 1, 2, 4, 8, 16, 32 cores each) on Turing and Navajo.

[Sudha247]

Thanks for adding the benchmarks @Firobe!

The config files have the correct benchmarks added to them. But, the benchmarks/mpl/bench directory has more benchmarks that are already present in sandmark. Perhaps we can remove the already available ones? You can see the existing parallel benchmarks in multicore-numerical and other directories with multicore-prefix.

[Firobe]

I think it should now be good to go!

[Sudha247]

Thanks for the updates. Looks good to me!

[shakthimaan]

Thanks for the review! LGTM.

[kayceesrk]

Do these benchmarks appear on the sandmark nightly results? @shakthimaan

If not, can you make them appear on the nightly results?

[Sudha247]

We should add a macro_bench tag for them to appear in sandmark-nightly, I think.

[kayceesrk]

Do we know that these benchmarks deserve macro_bech tag? In particular, do they run for enough time?

[kayceesrk]

One way to find out is to add the macro_bench tag and then observe the results on the nightly results and tune them appropriately.

[Sudha247]

All the benchmarks have a gt_100s tag, so I take it as they run for more than 100s at least for the one-core version. In that case, we can safely tag them as macro_bench.

[kayceesrk]

In addition to the missing macro_bench tag, the benchmarks are not in the right format to be visualized. In particular for a benchmark to be visualized,

1. The benchmark should have a serial version with the name foo.
2. The benchmark should have a parallel version with the name foo_multicore
   • IMPORTANT: Parallel version running on 1 domain is not the same as the sequential version.
3. The first argument (params or short_name) should be the number of domains. There can be other arguments that follow. When using short_name, the format should be <num_domains>_<other_arg> with an _ separating the number of domains and other arguments.

I also think this is the reason why graph500 results also don't appear in the nightly results.

It appears that this part of the codebase needs a little bit of love and attention. Someone should:

1. Review all of the current parallel benchmarks to see whether each of the benchmarks that appear to be tagged as macro_bench does appear in the nightly run.
2. Make an issue with all the benchmarks tagged as macro_bench that doesn't appear in the nightly runs.
3. Triage each one to identify what the problem is.
4. Update the documentation so that the contributors know what is the expectation (see previous list).
   • Perhaps convert this to a checklist that the contributors and the reviewers can use.
   • Better built tooling that can catch this.

Unfortunately, the current state is less than ideal. We don't yet have the tools/process to review and catch these before this PR and similar others are merged.

[punchagan]

Thanks for the review of the benchmarks and the notes. I have not looked at the new mpl benchmarks in question in this PR, but I agree that the current state of the benchmarks should be improved.

I also think this is the reason why graph500 results also don't appear in the nightly results.

graph500 results were disabled because the benchmark implementation was not scalable, and we have disabled the results from showing up in the UI until we fix the scalability.

Building a checklist for the contributors and/or reviewers would be a good first step, and then some tooling around making sure at least the easily verifiable checklist requirements are being followed.

[kayceesrk]

Building a checklist for the contributors and/or reviewers would be a good first step, and then some tooling around making sure at least the easily verifiable checklist requirements are being followed.

Tooling here is a bit tricky. One could possibly build this as a jq script that checks for a given benchmark whether serial version exists followed by parallel benchmark named appropriately, etc.

graph500 results were disabled because the benchmark implementation was not scalable, and we have disabled the results from showing up in the UI until we fix the scalability.

Thanks for the clarification here.

[Firobe]

Okay, I'm going to update the mpl benchmarks with the proper tags and naming conventions, and document the expectations w.r.t. naming KC listed in the README.

[kayceesrk]

Thanks @Firobe 👍

[Firobe]

See #439