charm icon indicating copy to clipboard operation
charm copied to clipboard

Cray Shasta support for Charm++ 6.10.x ?

Open marcodelapierre opened this issue 2 years ago • 8 comments

Hi, I was wondering whether you have plans to add support for Shasta on the previous Charm++ version 6.10.x.

Context: I have noticed support has been added through PRs #3497 and #3499 and is available in 7.0.0.

However, there are issues with the Molecular Dynamics code NAMD, which uses Charm++. The latest NAMD version, 2.14, only builds with Charm++ 6.x, while it crashes with 7.0.0 due to what seem API changes in Charm++. Development versions of NAMD also seem not to have support for 7.0.0 yet.

Thanks in advance, Marco

marcodelapierre avatar Dec 01 '21 07:12 marcodelapierre

That should be possible, I'll investigate and see if we can put out a point release for 6.10.x to add support there.

Separately, the development version of NAMD (both master and devel) at https://gitlab.com/tcbgUIUC/namd should work with 7.0.0 and the current main of Charm++ as far as I know. If you're having issues building that, then there's probably some bug that we need to fix. Can you provide more details about whatever error you're seeing with the development build?

rbuch avatar Dec 02 '21 05:12 rbuch

Thanks for the feedback @rbuch! I do have a colleague who is looking into patching 6.10.x for Shasta, I might join to this conversation when he's happy with his solution.

Regarding issues in building with Charm 7.0.0, note I was using NAMD release 2.14, not master/devel. In this context, I did get errors that seemed related to API changes in Charm, for instance:

src/NamdCentLB.C:597:24: error: 'struct BaseLB::LDStats' has no member named 'n_objs'
  597 |   for (j=0; j < stats->n_objs; j++) {

Note how I am using Spack to install NAMD, Charm and their dependencies.

[edit]: I am getting the same errors also when trying to build NAMD version 2.15a1, and the master branch (as of today, 8 December)

marcodelapierre avatar Dec 08 '21 03:12 marcodelapierre

Thanks for the feedback @rbuch! I do have a colleague who is looking into patching 6.10.x for Shasta, I might join to this conversation when he's happy with his solution.

Regarding issues in building with Charm 7.0.0, note I was using NAMD release 2.14, not master/devel. In this context, I did get errors that seemed related to API changes in Charm, for instance:

src/NamdCentLB.C:597:24: error: 'struct BaseLB::LDStats' has no member named 'n_objs'
  597 |   for (j=0; j < stats->n_objs; j++) {

Note how I am using Spack to install NAMD, Charm and their dependencies.

[edit]: I am getting the same errors also when trying to build NAMD version 2.15a1, and the master branch (as of today, 8 December)

Ah, okay, I misunderstood, then. It's expected that 2.14 and the current alpha of 2.15 won't build with 7.0.0, upcoming releases of NAMD should, however.

I'm working on creating a 6.10.3 release to add Shasta support, I'll update this issue when I have a candidate solution for that.

rbuch avatar Dec 08 '21 21:12 rbuch

Many thanks for the update Ronak

marcodelapierre avatar Dec 09 '21 01:12 marcodelapierre

Pinging my colleague Sam @halfflat , who was working on a Shasta patch as well

marcodelapierre avatar Dec 09 '21 01:12 marcodelapierre

I've made a branch (https://github.com/halfflat/charm/tree/shasta-on-6.10.2) which pulls in the #3032, #3497 and #3499 PRs onto v6.10.2, resolving conflicts and removing, as much as I noticed, any 7.0.0-specific code. I've only confirmed that the mpi-crayshatsa and ofi-crayshasta targets build.

Would it be useful to submit this as a PR?

halfflat avatar Dec 09 '21 02:12 halfflat

I've made a branch (https://github.com/halfflat/charm/tree/shasta-on-6.10.2) which pulls in the #3032, #3497 and #3499 PRs onto v6.10.2, resolving conflicts and removing, as much as I noticed, any 7.0.0-specific code. I've only confirmed that the mpi-crayshatsa and ofi-crayshasta targets build.

Would it be useful to submit this as a PR?

Your branch looks good and should be usable for your purposes, but it doesn't work for the CMake-based build (using ./buildcmake). It's not the default in 6.10.x, so it's not critical to have, but I'd like to have it working for the 6.10.3 release. That's where most of the complexity of backporting the Shasta support lies since that part has to be disentangled from the rest of the changes to the CMake build. That's what I'm working on right now, but until I finish that and put out the 6.10.3 release, you can use your branch.

rbuch avatar Dec 09 '21 17:12 rbuch

Thanks for taking a look at it! The other thing I was hoping to add in a separate patch is an option to make +ofi_runtime_tcp on by default, so that we don't have to have people add it at invocation time. What do you think?

halfflat avatar Dec 10 '21 01:12 halfflat