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

Add Parallel benchmark mergesort #336

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

ernestmusong
Copy link
Contributor

No description provided.

@ernestmusong
Copy link
Contributor Author

Hi @Sudha247 please review this PR for errors and possible fix. Thanks

@Sudha247
Copy link
Contributor

I'm on my phone, I'll be able to give a better review when I'm back at my computer. But from what I see in the CI failure, it looks like the Common module is missing. It needs to be imported from the original repo. An easy way to do that would be copying the file to the directory you are currently working on.

@ernestmusong
Copy link
Contributor Author

Hello @sudha after a series of build failures in PR #336 I see that "CCArray_slice module is used in mergesort.ml and common.ml which are inter-dependent. However I found out that "Array_slice" module has been removed as referenced here
https://github.com/c-cube/ocaml-containers
The Docs says "There is no replacement at the moment. Please tell us if you need this to be turned into a sub-library."
I assume that CCArray_Slice and Array_Slice are the same and if thus what is the way forward to fix the issue?

@Sudha247
Copy link
Contributor

Thanks for the PR @ernestmusong! Good catch! It appears CCArray_slice was removed from the containers library, but it's still used in the original benchmark.

A quick way to test would be to use an older version of containers to validate the benchmark run. Long term solution I think is to rewrite the benchmark without using CCArray_slice module.

@ernestmusong
Copy link
Contributor Author

Hi @Sudha247 I've downgraded the version domainslib and containers as specified in the repo but still get this error when run:

"The module AS is an alias for module CCArray_slice, which is missing
make: *** [Makefile:215: ocaml-versions/5.0.0+stable.bench] Error 1"

@ernestmusong
Copy link
Contributor Author

Hello @Sudha247 after a deep comparison between CCArray and CCArray_slice as used in mergesort and qsort, there exist common methods such as get, set, and lenght as used in the benchmarks. However, qsort seems easier to fix because it contains just "CCArray_slice.full" that is not present in CCArray. Further research could probably find a replacement for CCArray_slice.full. to make this benchmark running.
See here https://github.com/ckoparkar/ocaml-benchmarks/blob/master/qsort.ml

On the other hand, mergesort contains additional methods which have no equivalence in CCArray such as CCArray_slice.sub.

Please I would love to add qsort instead of mergesort as soon as I get a replacement for CCArray_slice.full

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