search

acts-project / algebra-plugins

Mozilla Public License 2.0
3 stars 10 forks source link

ref: added a mat_mul function to the matrix actor #81

Closed niermann999 closed 1 year ago

niermann999 commented 1 year ago

Added a mat_mul function to the matrix actor, since algebra libraries define operator* differently. Also refactored some of the Eigen implementation to use more expressions.

niermann999 commented 1 year ago

@beomki-yeo sorry, that I have to ask with a time constraint, but could you have an ultra fast look at this, please ?

beomki-yeo commented 1 year ago

Looks good to me. As there are many changes, could you try to run detray unit tests with this PR?

niermann999 commented 1 year ago

I am also happy to take some things back later on. How do i make detray pick up my algebra-plugins branch?

niermann999 commented 1 year ago

Damn, I cannot reproduce this locally

beomki-yeo commented 1 year ago

You can simply replace the URL in the extern/algebra-plugins/CMakeLists.txt. You will need to clean the build directory before running cmake command

# Declare where to get Algebra Plugins from.
set( DETRAY_ALGEBRA_PLUGINS_SOURCE
     #"URL;https://github.com/acts-project/algebra-plugins/archive/refs/tags/v0.14.0.tar.gz;URL_MD5;ecf9c90f64f41677803a6790eb2ececb"
     "GIT_REPOSITORY;https://github.com/niermann999/algebra-plugins;GIT_TAG;ref-add-mat_mul"
   CACHE STRING "Source for Algebra Plugins, when built as part of this project" )
niermann999 commented 1 year ago

You can simply replace the URL in the extern/algebra-plugins/CMakeLists.txt. You will need to clean the build directory before running cmake command

# Declare where to get Algebra Plugins from.
set( DETRAY_ALGEBRA_PLUGINS_SOURCE
     #"URL;https://github.com/acts-project/algebra-plugins/archive/refs/tags/v0.14.0.tar.gz;URL_MD5;ecf9c90f64f41677803a6790eb2ececb"
     "GIT_REPOSITORY;https://github.com/niermann999/algebra-plugins;GIT_TAG;ref-add-mat_mul"
   CACHE STRING "Source for Algebra Plugins, when built as part of this project" )

Cool, thanks! No worries now, I won't be able to finish this before tomorrow

niermann999 commented 1 year ago

So, let's do this right over the next few days :)

niermann999 commented 1 year ago

I think, the consensus was that Fastor #78 should be wrapped or get an operator overload (if possible) instead of using a mat_mul function. Closing this