search

Ivanlh20 / multem

MULTEM is a powerful and advanced collection of C++ routines with CUDA support, designed to perform efficient and accurate multislice simulations for various TEM experiments such as HRTEM, STEM, ISTEM, ED, PED, CBED, ADF-TEM, ABF-HC, EFTEM, and EELS.
GNU General Public License v3.0
65 stars 26 forks source link

Multem as a shared library #46

Closed jmp1985 closed 10 months ago

jmp1985 commented 3 years ago

This pull request refactors the multem core to be a shared library. It also adds python bindings. Here is a summary of what the pull request contains.

  1. I've reorganised the folders as follows:

    multem - c++/cuda code for shared library multem-matlab - c++/matlab code for matlab bindings multem-python - c++/python code for python bindings multem-gui - The multem GUI code

  2. The multem shared library has an interface that only uses STL components (e.g. I have tried my best to hide CUDA stuff to make software that uses the library easier to compile).

  3. The consequence of 2 is that I needed to modify a few classes to use "stl::vector" instead of "host_vector". This appeared to have very few consequences in the vast majority of cases.

  4. The one place I had a problem doing this was in Output_Multislice class. In order to avoid changing to many things, I settled on making a proxy interface class which copies the data into an STL vector as the interface but it may be worth revisiting to refactor to remove this proxy interface.

  5. I've added the python bindings for all the stuff exported by the matlab bindings and I've added some tests to ensure stuff runs. However, I want to add some more tests to ensure the output is always the same as before. In general, I think that we could do with writing a few more tests to ensure that big changes don't end up breaking things!

  6. I've made sure that the matlab bindings also compile and I've modified them to build using cmake

  7. Everything can be built with cmake which I think makes it more portable and easier to understand the build process.

To build you need to do something like this:

# Setup environment
python3.7 -m venv env
. ./env/bin/activate
python -m pip install cmake

# Build the Multem shared library
mkdir multem/build
cd multem/build
cmake ../ -DCUDA_TOOLKIT_ROOT_DIR=/usr/local/cuda-10.2 -DCMAKE_CUDA_COMPILER=/usr/local/cuda-10.2/bin/nvcc -DCMAKE_INSTALL_PREFIX=../../env
make
make install
cd ../../

# Build python bindings
cd multem-python
git submodule update --init --recursive
export MULTEMDIR=../env
pip install -r requirements.txt
pip install -e .
cd ..

# Build matlab bindings
cd multem-python
git submodule update --init --recursive
mkdir build
cd build
cmake ../ -DMULTEM_ROOT=../env
make
make install
cd ../..

Using cmake it should be possible to streamline the build process to build everything with a single command or alternatively we could have separate repositories for the core library and the language bindings so I thought I would wait to get feedback before doing either of those things.

Also I am treating this as a draft pull request because I still have some things I want to do but I would like your feedback in the meantime to ensure I am on the right track.

  1. I want to write some more tests to ensure that the output is exactly the same as before in all instances (I think it would be useful to have more tests anyway).
  2. I am not 100% happy with the way I handled exporting the Output_Multislice class in the shared library so this may need some discussion.
  3. I also need to ensure the gui also builds with the shared library but I haven't had a go at this yet.
dnjohnstone commented 3 years ago

@jmp1985 I'd be really interested in this coming from the perspective of what we've been doing in https://github.com/pyxem we've got some very naive simulations for use in some template matching workflows, but we've long wanted to find a multislice code with some nice python bindings and this would be ideal. Do you know if this is likely to move forwards at all? FYI @pc494

jmp1985 commented 3 years ago

@pc494 Thanks for your interest! This is currently ongoing work, the main author of multem @Ivanlh20 is working on some updates which will need to be merged into this pull request and after that I am planning to write some additional tests (sometime in early April) but we are hoping to merge this in the not too distant future. In the meantime, you are welcome to checkout this branch (https://github.com/jmp1985/MULTEM/tree/make_shared_library) and the code should work.

jmp1985 commented 10 months ago

Hi Ivan

I didn't realise you were going to merge this into master right away.

Given that you will be trying to merge multem_v3, perhaps we should try and get those changes from multem_v3 merged first and then merge my changes on top of those? Alternatively, we could merge this into a separate branch until we are happy that everything is ready to release.

Best wishes

James

On 30/10/2023 11:11, Ivan wrote:

Merged #46 https://github.com/Ivanlh20/multem/pull/46 into master.

— Reply to this email directly, view it on GitHub https://github.com/Ivanlh20/multem/pull/46#event-10806164024, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARDKYP55QJM6QXPVWPK6DLYB6DPFAVCNFSM4WRRQN22U5DIOJSWCZC7NNSXTWQAEJEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW4OZRGA4DANRRGY2DAMRU. You are receiving this because you were mentioned.Message ID: @.***>

-- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom

--------------0x8HhCRAjn4pWaRMseNg0Ras Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

<!DOCTYPE html>

Hi Ivan

I didn't realise you were going to merge this into master right away. 

Given that you will be trying to merge multem_v3, perhaps we should try and get those changes from multem_v3 merged first and then merge my changes on top of those? Alternatively, we could merge this into a separate branch until we are happy that everything is ready to release.

Best wishes

James

On 30/10/2023 11:11, Ivan wrote:

Merged #46 into master.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: <Ivanlh20/multem/pull/46/issue_event/10806164024@github.com>

 

-- 

This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail.
Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd.
Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message.
Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom
 

--------------0x8HhCRAjn4pWaRMseNg0Ras--