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

Test: make CI test fails with warnings #4493

Merged
merged 5 commits into from
Jul 3, 2024

Conversation

WHUweiqingzhou
Copy link
Collaborator

The List of Changes

  1. Update to commit 494fca4, there are 58 warnings in github CI tests:
Warning: NG]    58 test cases out of 1227 failed.
1: 101_PW_15_lowz
1: 101_PW_15_paw
1: 101_PW_blps_pseudopots
1: 101_PW_upf201_uspp_NaCl
1: 101_PW_Coulomb
1: 102_PW_BPCG
1: 103_PW_CF_CS_S1_smallg
1: 103_PW_CF_CS_S2_smallg
1: 104_PW_NC_magnetic
1: 105_PW_FD_smearing
1: 105_PW_GA_smearing
1: 107_PW_outWfcR
1: 108_PW_RE
1: 108_PW_RE_MB
1: 109_PW_CR_fix_a
1: 109_PW_CR_fix_ab
1: 109_PW_CR_fix_abc
1: 109_PW_CR_fix_ac
1: 109_PW_CR_fix_b
1: 109_PW_CR_fix_bc
1: 109_PW_CR_fix_c
1: 109_PW_CR_moveatoms
1: 110_PW_SY_symmetry_LiRh
1: 111_PW_elec_add
1: 111_PW_elec_minus
1: 111_PW_S2_elec_add
1: 111_PW_S2_elec_minus
1: 115_PW_sol_H2
1: 116_PW_scan_Si2_nspin2
1: 120_PW_KP_MD_NPT
1: 120_PW_KP_MD_NVT
1: 133_PW_DJ_PK
1: 150_PW_15_CR_VDW3
1: 170_PW_MD_1O
1: 170_PW_MD_2O
1: 180_PW_SDFT_10S_M
1: 180_PW_SDFT_10S_P
1: 181_PW_SDFT_5D10S
1: 182_PW_SDFT_ALL
1: 183_PW_MD_SDFT_10S
1: 183_PW_MD_SDFT_5D10S
1: 183_PW_MD_SDFT_ALL
1: 184_PW_BNDPAR_SDFT_10S
1: 184_PW_BNDPAR_SDFT_5D10S
1: 184_PW_KPAR_SDFT_ALL
1: 185_PW_SDFT_10D10S_METHD2
1: 185_PW_SDFT_10S_METHD2
1: 186_PW_SDOS_10D10S
1: 186_PW_SNLKG_10D10S
1: 250_NO_KP_CR_VDW3ABC
1: 281_NO_KP_HSE
1: 286_NO_KP_CR_HSE
1: 384_NO_GO_S1_HSE_loop0_PU
1: 601_NO_TDDFT_H2_restart
1: 601_NO_TDDFT_CO
1: 601_NO_TDDFT_CO_occ
1: 803_PW_LT_bcc
1: 806_PW_LT_st

and 3 warnings for GPU test:

102_PW_CG_GPU
102_PW_DA_davidson_GPU
102_PW_BPCG_GPU
  1. this PR add threshold file in these 58+3 files.
  2. Modify Autotest.sh and make CI test fail once fatal error or warning happen. (Before this PR, CI test fails only when fatal error occurs.)

Please Notice that

  1. After this PR, warnings are also unaccepted as same as the fatal errors. Due to the potential numerical instability inherent to ABACUS, modifications might lead to some unrelated numerical fluctuations, and lead to warning. If a PR fails due to warning. I will suggest that PR should be reviewed by at least 3 developers. If all reviewers think PR is OK. You can update corresponding references and make your PR pass.
  2. PR test: support reading threshold from file 'threshold' in integrate test #4450 from @pxlxingliang enable one can prepare a threshold in each cases directory. For numerically unstable test cases—those that yield different results on different runs, one can establish a tolerance level to relax the error conditions by editing threshold file.

@WHUweiqingzhou
Copy link
Collaborator Author

While updating the thresholds for 58 warnings, I encountered a serious bug in the current Autotest workflow. The current mechanism for determining warnings and fatal errors is flawed. For instance, let's take an example that has three indices: energy, force, and stress.

The correct logic should be: if none of the three indices is a fatal error, and there is at least one warning, then it should be classified as a warning; however, if any one of the indices is a fatal error, then the whole test should be classified as a fatal error.

But the current logic is: as soon as any index is deemed as a warning, the workflow exits immediately without checking the subsequent indices, even though they may be completely wrong.

In reality, we actually have 21 force indices with fatal errors hidden behind a warning for the total energy index:

Warning: NG]    21 test cases out of 1405 failed.
1: 101_PW_15_paw
1: 108_PW_RE
1: 109_PW_CR_fix_a
1: 109_PW_CR_fix_ab
1: 109_PW_CR_fix_abc
1: 109_PW_CR_fix_ac
1: 109_PW_CR_fix_b
1: 109_PW_CR_fix_bc
1: 109_PW_CR_fix_c
1: 109_PW_CR_moveatoms
1: 111_PW_S2_elec_add
1: 116_PW_scan_Si2_nspin2
1: 120_PW_KP_MD_NPT
1: 120_PW_KP_MD_NVT
1: 170_PW_MD_1O
1: 170_PW_MD_2O
1: 180_PW_SDFT_10S_M
1: 181_PW_SDFT_5D10S
1: 183_PW_MD_SDFT_ALL
1: 184_PW_BNDPAR_SDFT_10S
1: 184_PW_BNDPAR_SDFT_5D10S

@WHUweiqingzhou
Copy link
Collaborator Author

WHUweiqingzhou commented Jul 3, 2024

I find #4503 may induce 3 new warnings:

802_PW_LT_fcc
807_PW_LT_bct
810_PW_LT_fco

@jinzx10 could you have a look?
Anyway, I will add threshold for these 3 cases to pass CI tests.

So current changed file is 66: 2(Autotest.sh) + 58 + 3 + 3

@jinzx10
Copy link
Collaborator

jinzx10 commented Jul 3, 2024

I find #4503 may induce 3 new warnings:

802_PW_LT_fcc
807_PW_LT_bct
810_PW_LT_fco

@jinzx10 could you have a look? Anyway, I will add threshold for these 3 cases to pass CI tests.

So current changed file is 66: 2(Autotest.sh) + 58 + 3 + 3

It turns out that the failure of stress comparison has been a long-standing problem muted in the past. I traced all the way back to #3675 (version update to v3.5.4) and the comparison already failed back then. To nail down the specific PR that introduces the problem, I think we need to resort to developers that are familiar with the recent changes in the stress calculation in PW.

I think the PR that first exposed this problem was #4496 :

image

#4496 was merged on July 1. Four other PRs were merged that day afterwards, but their checks were done prior to the merge of #4496 , therefore no warnings from the stress test were displayed in their respective workflows.

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.

None yet

4 participants