cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
218 stars 116 forks source link

Merge changes made for Frontier #934

Closed Trzs closed 8 months ago

Trzs commented 11 months ago

Big merge to bring all changes made for Frontier back to the main branch.

WARNING: This merge requires Kokkos 4.0 or later!

Changes for Kokkos 4.0

Changes for nxmx_writer

Other Changes

nksauter commented 11 months ago

@dermen @Trzs Bugreport running simtbx/run_tests.py on Perlmutter: libtbx.python "modules/cctbx_project/simtbx/diffBragg/tests/tst_diffBragg_hopper_refine_Fhkl.py" --scale .2 [FAIL] 622.0s Time: 622.02 Return code: 1 OKs: 0 Standard error: Traceback (most recent call last): File "modules/cctbx_project/simtbx/diffBragg/tests/tst_diffBragg_hopper_refine_Fhkl.py", line 334, in modelers = load_inputs(df, P, exper_key="opt_exp_name", refls_key=refl_col) File "modules/cctbx_project/simtbx/diffBragg/hopper_ensemble_utils.py", line 577, in load_inputs work_distribution = prep_dataframe(pandas_table, refls_key) File "modules/cctbx_project/simtbx/diffBragg/prep_stage2_input.py", line 56, in prep_dataframe R = Rall.select(Rall['id'] == int(expt_id)) KeyError: "Unknown column 'id'"

dermen commented 11 months ago

One thing Id like to do before this goes in is to check carefully against this branch which is a merge of nxmx_writer_experimental and master after some changes from @mewall :

https://github.com/cctbx/cctbx_project/tree/diffuse

dermen commented 11 months ago

Btw, is there a recommended way to switch to Kokkos 4 ? Does git pull from the kokkos repos suffice ?

Trzs commented 11 months ago

Btw, is there a recommended way to switch to Kokkos 4 ? Does git pull from the kokkos repos suffice ?

The currently best way is to manually checkout kokkos 4.0 or kokkos 4.1 (and kokkos-kernels 4.0/4.1 respectively). Then you need to remove the build files, checkout the frontier_merge_candidate branch and run make in BUILD.

Detailed list:

Also, after the installation, you might get an error about mdspan. To fix this, go to kokkos_build and run "ccmake ../kokkos". Then hit "T" to toggle advanced mode and search for "Kokkos_ENABLE_IMPL_MDSPAN" and set it to ON. Then "c" for configure and "g" to generate. Then just "make install"

If you get an error about lunus, try to checkout the "lunus/update_kokkos" branch

mewall commented 11 months ago

If you get an error about lunus, try to checkout the >"lunus/update_kokkos" branch

This branch has now been merged into lunus master.


From: Felix Wittwer @.> Sent: Monday, November 6, 2023 4:56 PM To: cctbx/cctbx_project @.> Cc: Wall, Michael E @.>; Mention @.> Subject: [EXTERNAL] Re: [cctbx/cctbx_project] Merge changes made for Frontier (PR #934)

Btw, is there a recommended way to switch to Kokkos 4 ? Does git pull from the kokkos repos suffice ?

The currently best way is to manually checkout kokkos 4.0 or kokkos 4.1 (and kokkos-kernels 4.0/4.1 respectively). Then you need to remove the build files, checkout this branch and run make in BUILD.

Full list:

Also, after the installation, you might get an error about mdspan. To fix this, go to kokkos_build and run "ccmake ../kokkos". Then hit "T" to toggle advanced mode and search for "Kokkos_ENABLE_IMPL_MDSPAN" and set it to ON. Then "c" for configure and "g" to generate. Then just "make install"

If you get an error about lunus, try to checkout the "lunus/update_kokkos" branch

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/cctbx/cctbx_project/pull/934*issuecomment-1797051270__;Iw!!Bt8fGhp8LhKGRg!FG838veJlYof_L-7hMWtam9cAlC9haNFRRcxSytTTUGdy3kroNjXWpvaCIm0GMKpMEkNxuXoQ2bOdvW0DJtfh4SS$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AA67VEOT2PHCEBR6UBNPPKDYDF2L3AVCNFSM6AAAAAA6TBNS32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJXGA2TCMRXGA__;!!Bt8fGhp8LhKGRg!FG838veJlYof_L-7hMWtam9cAlC9haNFRRcxSytTTUGdy3kroNjXWpvaCIm0GMKpMEkNxuXoQ2bOdvW0DISzWnzl$. You are receiving this because you were mentioned.Message ID: @.***>

Baharis commented 11 months ago

Here is a full list of instructions I ran (after cloning all the modules) to move to kokkos 4.1 for anyone's future reference. Some of them might not be necessary, the latter half can be avoided by moving to the lunus branch, but this is exactly what worked for me :)

cd cctbx/modules/kokkos
git checkout release-candidate-4.1.00
cd ../kokkos-kernels
git checkout release-candidate-4.1.00
cd ..
rm -r kokkos_build/
rm -r kokkos-kernels/build/
cd $BUILD
make
cd $MODULES/kokkos_build/
ccmake ../kokkos
# Update the flag as discussed by Felix: "T" -> "Kokkos_ENABLE_IMPL_MDSPAN" ON -> "c", "e", "g".
make install
vi $MODULES/lunus/lunus/kokkos/SConscript
# Replace all instances of “c++11” or “c++14” with “c++17”. 
cd $BUILD
make

Edit: since lunus branch has been merged, the last 4 lines should be irrelevant.

dermen commented 11 months ago

@Baharis , any idea what this change is for, I cant remember that one

diff --git a/simtbx/command_line/make_input_file.py b/simtbx/command_line/make_input_file.py
index 3363374f71..3229cd1b53 100644
--- a/simtbx/command_line/make_input_file.py
+++ b/simtbx/command_line/make_input_file.py
@@ -72,6 +72,7 @@ def split_stills_expts(expt_f, refl_f, split_dir, write=False):
             new_expt_name = os.path.join(split_dir, unique_tag)
         new_refl_name = new_expt_name.replace(".expt", ".refl")
         refls = R.select(R['id'] == i_expt)
+        refls.reset_ids()

         if write:
             one_exp_El.as_file(new_expt_name)
dermen commented 10 months ago

things seem to look ok from my end ...

dermen commented 10 months ago

all simtbx tests now pass on my end (@nksauter )

dermen commented 9 months ago

As far as Im concerned, this is good to merge in to master

nksauter commented 9 months ago

All simtbx and LS49 tests pass with my Kokkos 4 build on Perlmutter. Awaiting the Azure results now.