mlx icon indicating copy to clipboard operation
mlx copied to clipboard

feat: add logicalAnd and logicalOR

Open NripeshN opened this issue 1 year ago • 5 comments

Proposed changes

Added logicalAnd and logicalOR closes #362

Checklist

Put an x in the boxes that apply.

  • [x] I have read the CONTRIBUTING document
  • [x] I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] I have updated the necessary documentation (if needed)

NripeshN avatar Jan 06 '24 10:01 NripeshN

Hi @awni, The code is not compiling for some reason when pip install -e . is run This is the error

      ld: Undefined symbols:
        mlx::core::LogicalAnd::eval_gpu(std::__1::vector<mlx::core::array, std::__1::allocator<mlx::core::array>> const&, mlx::core::array&), referenced from:
            vtable for mlx::core::LogicalAnd in primitives.cpp.o
        mlx::core::LogicalOr::eval_gpu(std::__1::vector<mlx::core::array, std::__1::allocator<mlx::core::array>> const&, mlx::core::array&), referenced from:
            vtable for mlx::core::LogicalOr in primitives.cpp.o
      clang: error: linker command failed with exit code 1 (use -v to see invocation)```
      
      

NripeshN avatar Jan 06 '24 10:01 NripeshN

The code is not compiling for some reason when pip install -e . is run

🤔 Were you able to get it to build using Cmake directly?

awni avatar Jan 06 '24 13:01 awni

I'm not able to repro the build failure. It build fine for me 😅 ... maybe try clearing out your build caches..

awni avatar Jan 06 '24 13:01 awni

Does it build for you now?

awni avatar Jan 06 '24 18:01 awni

Does it build for you now?

Yup it builds fine now and all tests pass

NripeshN avatar Jan 06 '24 18:01 NripeshN

Thanks for the updates! I left a few more comments inline.

Also thanks for the C++ overloads, those will be nice to have. I was also hoping you would include the __and__ and __or__ python overloads in python/src/array.cpp. Wdyt?

Yup I have added the overloads

NripeshN avatar Jan 07 '24 09:01 NripeshN