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

OCaml 5.2 support #657

Closed
kit-ty-kate opened this issue Feb 22, 2024 · 5 comments
Closed

OCaml 5.2 support #657

kit-ty-kate opened this issue Feb 22, 2024 · 5 comments
Labels

Comments

@kit-ty-kate
Copy link

OCaml 5.2 added support for float16 bigarrays and owl-base fails with:

#=== ERROR while compiling owl-base.1.1 =======================================#
# context              2.2.0~beta2~dev | linux/x86_64 | ocaml-variants.5.2.0+trunk | file:https:///home/opam/opam-repository
# path                 ~/.opam/5.2/.opam-switch/build/owl-base.1.1
# command              ~/.opam/5.2/bin/dune build -p owl-base -j 1
# exit-code            1
# env-file             ~/.opam/log/owl-base-19-47ea7a.env
# output-file          ~/.opam/log/owl-base-19-47ea7a.out
### output ###
# (cd _build/default && /home/opam/.opam/5.2/bin/ocamlc.opt -w -40 -g -bin-annot -I src/base/.owl_base.objs/byte -I /home/opam/.opam/5.2/lib/ocaml/unix -no-alias-deps -o src/base/.owl_base.objs/byte/owl_utils_ndarray.cmo -c -impl src/base/owl_utils_ndarray.ml)
# File "src/base/misc/owl_utils_ndarray.ml", lines 9-22, characters 56-87:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# 
# File "src/base/misc/owl_utils_ndarray.ml", lines 26-41, characters 56-86:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# (cd _build/default && /home/opam/.opam/5.2/bin/ocamlc.opt -w -40 -g -bin-annot -I src/base/.owl_base.objs/byte -I /home/opam/.opam/5.2/lib/ocaml/unix -intf-suffix .ml -no-alias-deps -o src/base/.owl_base.objs/byte/owl_const.cmo -c -impl src/base/owl_const.ml)
# File "src/base/core/owl_const.ml", lines 48-61, characters 40-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# 
# File "src/base/core/owl_const.ml", lines 64-77, characters 39-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# 
# File "src/base/core/owl_const.ml", lines 80-93, characters 43-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# (cd _build/default && /home/opam/.opam/5.2/bin/ocamlopt.opt -w -40 -g -I src/base/.owl_base.objs/byte -I src/base/.owl_base.objs/native -I /home/opam/.opam/5.2/lib/ocaml/unix -intf-suffix .ml -no-alias-deps -o src/base/.owl_base.objs/native/owl_const.cmx -c -impl src/base/owl_const.ml)
# File "src/base/core/owl_const.ml", lines 48-61, characters 40-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# 
# File "src/base/core/owl_const.ml", lines 64-77, characters 39-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
# 
# File "src/base/core/owl_const.ml", lines 80-93, characters 43-28:
# Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
# Here is an example of a case that is not matched:
# Float16
@jzstark jzstark added the bug label Mar 7, 2024
@tmcgilchrist
Copy link

tmcgilchrist commented May 17, 2024

In addition to the missing Float16 pattern matches the C code has warnings from the K&R C style function declarations and incompatible pointer types.

https://github.com/tmcgilchrist/owl/tree/ocaml_5_2 has basic fix for pattern matches in ef69ebe and silenced K&R C warnings in c3a3582

Haven't looked into the last warnings about incompatible pointer types.

opam exec -- dune build @all
In file included from src/owl/core/owl_ndarray_contract_stub.c:17:
src/owl/core/owl_ndarray_contract_impl.h:88:9: warning: incompatible pointer types assigning to 'int64_t *' (aka 'long long *') from 'intnat[]' (aka 'long[]') [-Wincompatible-pointer-types]
  cp->n = X->dim;
        ^ ~~~~~~
In file included from src/owl/core/owl_ndarray_contract_stub.c:26:
src/owl/core/owl_ndarray_contract_impl.h:88:9: warning: incompatible pointer types assigning to 'int64_t *' (aka 'long long *') from 'intnat[]' (aka 'long[]') [-Wincompatible-pointer-types]
  cp->n = X->dim;
        ^ ~~~~~~
In file included from src/owl/core/owl_ndarray_contract_stub.c:35:
src/owl/core/owl_ndarray_contract_impl.h:88:9: warning: incompatible pointer types assigning to 'int64_t *' (aka 'long long *') from 'intnat[]' (aka 'long[]') [-Wincompatible-pointer-types]
  cp->n = X->dim;
        ^ ~~~~~~
In file included from src/owl/core/owl_ndarray_contract_stub.c:44:
src/owl/core/owl_ndarray_contract_impl.h:88:9: warning: incompatible pointer types assigning to 'int64_t *' (aka 'long long *') from 'intnat[]' (aka 'long[]') [-Wincompatible-pointer-types]
  cp->n = X->dim;
        ^ ~~~~~~
4 warnings generated.

I expect that proper Float16 support will need more work.

@mtk2xfugaku
Copy link

mtk2xfugaku commented May 19, 2024

Most scientific computing libraries support higher precision floating point operations (floats and doubles), to support float16 or bfloat16 or more exotic float8 the common BLAS functions has to be implemented with reduced precision. To support float16 now in Owl, the linear algebra operation can be done in higher precision (float32) by converting float16 array buffer to float32 array buffer and then stored back again in float16 array buffer.

Raising precision would cause some overhead but not that much, also accumulation in higher precision (float32) is also more stable. This is what ml_types library does for accumulation.

@tmcgilchrist
Copy link

@mtk2xfugaku thank you for the details, that makes sense.
I'm unlikely to have time to do that work, my main goal was to get it compiling for Sandmark GC regression tests.

@patrick-nicodemus
Copy link
Contributor

@tmcgilchrist I cloned your branch and ran the tests and everything passes. Based on your experience, what else do you feel needs to be done before this can be merged?

I expect that proper Float16 support will need more work.

In your opinion could this be left to a future project, or is this a necessary prerequisite before users of OCaml 5.2 can reliably use Owl?

@jzstark
Copy link
Collaborator

jzstark commented Sep 29, 2024

This compilation issue is fixed in the current Owl.

@jzstark jzstark closed this as completed Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants