Baunsgaard commented on pull request #1525: URL: https://github.com/apache/systemds/pull/1525#issuecomment-1036397251
@flo-l Hi! I like the PR! But i am worried about it modifying / extending the previous cpp files since it just muddle everything together, and does not lend itself to future extensions to our native bindings. If i am not mistaken (please correct me if you think am wrong) this current implementation means that we built in a dependency to mkl/blas and now SEAL in the same resource. Preferably i think it would be better if we separate these two dependencies such that: 1. We make a new header file for the SEAL dependency and build into new different .so /.dll filles for SEAL bindings in src/main/cpp/lib, perhaps called libsystemds_seal-Linux-x86_64.so and libsystemds_seal-Windows-AMD64.so. 2. We make a new Java file that binds to it (instead of extending src/main/java/org/apache/sysds/utils/NativeHelper.java) A suggestion is that we make a new directory inside main/cpp that separate the new addition entirely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
