srtee / lammps-USER-CONP2

updated constant potential plugin for LAMMPS
GNU General Public License v3.0
36 stars 16 forks source link

Updated installation process to hook in to LAMMPS' CMake process. #24

Closed emilyviolet closed 3 years ago

emilyviolet commented 3 years ago

This commit adds a shell-script to copy the USER-CONP2 source files and patch LAMMPS' CMakeLists.txt to automatically build and link the CONP2 module, including functionality to automatically find and/or build BLAS and LAPACK libs.

emilyviolet commented 3 years ago

This commit looks like it adds the same functionality as Shern's additions in the CTCMS forks of LAMMPS, but goes about it in a different way. Specifically, it keeps all the functionality within the USER-CONP2 repository and attempts to make as few changes to the LAMMPS source tree as possible during installation. This is not the only way to make a user-friendly installer and I'm of course open to doing things differently.

emilyviolet commented 3 years ago

Hi Shern,

Thanks for the feedback! These are all very good points. In order:

  1. Thanks for pointing that out, my bad for missing it. I've added the changes in the CTCMS repo to the patchfile and it compiles fine; are there any others I should know about?
  2. I've actually tested this and LAMMPS is smart enough to handle it properly: CMake doesn't compile the pppm_conp_intel files unless both USER-INTEL and USER-CONP2 are enabled, so I think we're safe there. There's a bunch of package-specific source files in the USER-INTEL folder (and in all the accelerator packages, too) for bonds, angles and kspace styles corresponding to optional packages, so I think this is the idiomatic way of handling accelerated styles. The same is true of the plain Make-based build system: it checks to see if the "parent" style class is already present in the install directory before building the intel-accelerated style.
  3. Just tested this, and I needed to update my install script to move Install.sh to the right directory, and removed the "intel" styles from Install.sh (since they're now handled by USER-INTEL's script). I also updated Depend.sh so it should handle loading and unloading the CONP2 package (e.g. unloading the right files when the user does make no-user-conp2). I've tested various permutations of enabling/disabling USER-INTEL, KSPACE, and USER-CONP2 and I think it should Just Work now.

Have a look at the new commit and let me know what you think.

Cheers,

Emily

srtee commented 3 years ago

Looks good! I'm just curious what will happen to the incl_pppm_intel_templates.cpp hack file with this different way of doing things. It's unlikely to have significant bad side effects though.

I'm curious how other users will take to the new installation process. On my part, I have usually kept all files in a USER-CONP2 folder (so Git can manage everything in that folder); moving the Intel files to USER-INTEL is a different and interesting way of handling things, which we should keep an eye on.