Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored into library and MD bin. #31

Merged
merged 6 commits into from
Feb 20, 2020
Merged

Conversation

frobnitzem
Copy link
Contributor

I'm trying to create a Monte Carlo program from this, and have the following roadmap in-mind:

  1. Break up cmake file into library and executable (to simplify work of adding multiple executables).
  2. Create a test for energy calculations.
  3. Build class/template-based input method for molecular topologies.
  4. Create simplified route to calculation of Delta E-values.

This is step 1. Not being a cmake expert, I don't know if bin/CMakeLists.txt is too verbose. I think it doesn't need Kokkos::kokkos under under target_link_libraries, but maybe some of the others are redundant too because of cmake's linking rules.

@streeve
Copy link
Member

streeve commented Feb 10, 2020

Thanks for putting time into this. I created an issue that includes your main description to track progress - #33. We can update that as we go and make this PR more specific to step 1

I've also added a PR only for the CMake target changes #32 - if you merge that in here the build should pass.

@streeve
Copy link
Member

streeve commented Feb 10, 2020

@junghans any comments on CMake style?

CMakeLists.txt Outdated Show resolved Hide resolved
@frobnitzem
Copy link
Contributor Author

Accomplishes step 1 in Feature Request #33. Merges suggestions from PR #32 (thanks @streeve).

bin/CMakeLists.txt is now particularly simple. I don't know what you want to name the library and MD executable, but cmake only allows one target to be named CabanaMD.

.travis.yml Outdated Show resolved Hide resolved
Copy link
Member

@streeve streeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes. Probably easiest to wait until #32 is merged; there's quite a bit shifting with the Kokkos/Cabana updates

CMakeLists.txt Outdated Show resolved Hide resolved
@@ -0,0 +1,4 @@

add_executable(MD main.cpp)
target_link_libraries(MD LINK_PUBLIC CabanaMD)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe cbnMD instead?

Copy link
Contributor Author

@frobnitzem frobnitzem Feb 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adopted.

.travis.yml Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
src/CabanaMD_config.hpp.cmakein Show resolved Hide resolved
@streeve
Copy link
Member

streeve commented Feb 11, 2020

@frobnitzem should be fine to update - the CMake is even simpler now

Copy link
Member

@streeve streeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Ready to merge, @frobnitzem?

@frobnitzem
Copy link
Contributor Author

Yes, I think this one is ready. Did I miss a checkbox somewhere?

@streeve
Copy link
Member

streeve commented Feb 20, 2020

Nope, just wanted to make sure

@streeve streeve merged commit 66a610a into ECP-copa:master Feb 20, 2020
@streeve streeve mentioned this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants