TEAR-ERC / tandem

A HPC DG method for 2D and 3D SEAS problems
BSD 3-Clause "New" or "Revised" License
18 stars 10 forks source link

version 1.2 #83

Open hpc4geo opened 1 month ago

hpc4geo commented 1 month ago

There is interest in rolling out something resembling a release.

We have many branches with various fixes, features out there.

If you would like a specific branch merged into main as part of the upcoming hackathon, please reply to this issue and indicate the branch name.

All branches identified will be merged into a new branch

dev/staging

By suggesting a branch, you are implicitly also assuming responsibility for testing, and verifying the correctness of the behavior of the change / feature that your suggested branch introduces into tandem. Your testing and verification must be conducted within the branch dev/staging. You will also be asked for brief overview of how you verified the correctness of the fix / feature so that the test procedure you used can be incorporated into a regression test suite (and or CI test suite) for future testing of tandem.

hpc4geo commented 1 month ago

@JeenaYun @Thomas-Ulrich @baroryan @yohaimagen @AliceGabriel

Thomas-Ulrich commented 1 month ago

https://github.com/TEAR-ERC/tandem/pull/82 -> maybe redo the gf chepointing tests (and integrate some of them in the test pipeline? https://github.com/TEAR-ERC/tandem/pull/81 -> could be tested but how? https://github.com/TEAR-ERC/tandem/pull/80 -> no testing needed. only additonnal log (compilation could be tested by #69) https://github.com/TEAR-ERC/tandem/pull/72 ->no testing needded. documentation https://github.com/TEAR-ERC/tandem/pull/69 -> no further testing needded (?). adds a workflow testing compilation. we already seen that this fixes GPUs https://github.com/TEAR-ERC/tandem/pull/67 -> hard to test. but we've made the same change with seissol documentation and it fixed the build of the readthedocs. https://github.com/TEAR-ERC/tandem/pull/57 -> no need test (code-formatting + associated github action). Need to be discussed if we want that (IMO does not hurt and was already applied by Carsten)

yohaimagen commented 1 month ago

78 -> Need to remove hardcoded parameters and test for the best initial searching value (will do this early next week, before merging into one branch).

yohaimagen commented 3 weeks ago

78 ready to be merged.

JeenaYun commented 3 weeks ago

59 -> 1) need to test whether the checkpoint directory is creating without a problem, 2) need to state a disclaimer in the document about the harmless issue in the output writing

65 -> double-check using the BP6 example

JeenaYun commented 3 weeks ago

dmay/r_and_s_error_report branch -> test by running some models that is known to crash and check how the errror is reported

JeenaYun commented 2 weeks ago

jyun/main_state_laws

hpc4geo commented 2 weeks ago

59 -> 1) need to test whether the checkpoint directory is creating without a problem, 2) need to state a disclaimer in the document about the harmless issue in the output writing #65 -> double-check using the BP6 example

@JeenaYun Is PR 65 required if I merge jyun/main_state_laws?

JeenaYun commented 2 weeks ago

59 -> 1) need to test whether the checkpoint directory is creating without a problem, 2) need to state a disclaimer in the document about the harmless issue in the output writing #65 -> double-check using the BP6 example

@JeenaYun Is PR 65 required if I merge jyun/main_state_laws?

Nope. jyun/main_state_laws contains everything in #65

hpc4geo commented 2 weeks ago

59 -> 1) need to test whether the checkpoint directory is creating without a problem, 2) need to state a disclaimer in the document about the harmless issue in the output writing #65 -> double-check using the BP6 example

@JeenaYun Is PR 65 required if I merge jyun/main_state_laws?

Nope. jyun/main_state_laws contains everything in #65

Great - thanks!

hpc4geo commented 2 weeks ago

@Thomas-Ulrich @JeenaYun @baroryan @yohaimagen @AliceGabriel

All these branches (except one) have been merged into dmay/staging. This branch is ready for testing.

Branch thomas/update_yateto https://github.com/TEAR-ERC/tandem/pull/81 has been excluded. Once testing of dmay/staging is complete I will merge thomas/update_yateto and thomas can test it.

AliceGabriel commented 1 week ago

Hi everyone, quick reminder of the pending tests so @hpc4geo can finish v1.1 It would be great to have finished this by the end of next week (well in time for the AGU poster). You can also email Dave directly. Thanks.

Thomas-Ulrich commented 3 days ago

Just to confirm that the GF checkpointing tests are still passing. For the records, I'm using the following scripts: https://github.com/TEAR-ERC/tandem/pull/51#issuecomment-1721236646 I had to modify test_checkpointing.sh to account for the new interface: test_checkpointing.txt

yohaimagen commented 3 days ago

Confirming that the new root finding is still working properly. I have tested it both point-wise and quantitatively with BP1.

Regarding point-wise testing, the test unit is ready to be pushed and works properly. @hpc4geo if you want it to be part of v1.2, I have it ready on a branch a few commits ahead of dmay/staging. The only changes, except for adding the test files themselves, are the following additions to theapp/CMakeLists.txt file. Let me know if it's okay to push it to dmay/staging or not.

--- a/app/CMakeLists.txt
+++ b/app/CMakeLists.txt
@@ -34,6 +34,16 @@ target_compile_options(test-scatter PRIVATE ${CPU_ARCH_FLAGS})
 target_include_directories(test-scatter PRIVATE ../external/)
 target_link_libraries(test-scatter PRIVATE tandem-lib)

+
+add_executable(test_0 test/t_0/test_DieterichRuinaAgeing.cpp)
+target_compile_options(test_0 PRIVATE ${CPU_ARCH_FLAGS})
+include_directories(test_0 "${CMAKE_SOURCE_DIR}/app/")
+include_directories(test_0 "${CMAKE_SOURCE_DIR}/src/")
+include_directories(test_0 "${CMAKE_BINARY_DIR}/app/")
+target_link_libraries(test_0 PRIVATE tandem-lib)
+set_target_properties(test_0 PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/app/test/t_0")
+
+