diff --git a/docs/developers_guide/input_validation.md b/docs/developers_guide/input_validation.md index e1bb64736e..8a80a809a5 100644 --- a/docs/developers_guide/input_validation.md +++ b/docs/developers_guide/input_validation.md @@ -37,6 +37,9 @@ References and operand types are checked when registrations are assembled. Errors identify the file, parameter, offending value, legal choices/rules, and its manual URL. All ranks receive the error before parameter broadcasts. +File-open failures, a missing `INPUT_PARAMETERS` header, unknown names and +duplicate parameters use the same broadcast-and-exit path. Reader code must +return these errors to `read_parameters` rather than exit only on rank zero. Normal execution and `--check-input` share this preparation boundary. Existing checks needing structure, build capabilities or calculation context continue at their existing boundary. diff --git a/source/source_io/module_parameter/read_input.cpp b/source/source_io/module_parameter/read_input.cpp index 85f4b150df..66f1b41048 100644 --- a/source/source_io/module_parameter/read_input.cpp +++ b/source/source_io/module_parameter/read_input.cpp @@ -351,8 +351,7 @@ std::string ReadInput::read_txt_input(Parameter& param, const std::string& filen if (!ifs) { - std::cout << " Can't find the INPUT file." << std::endl; - ModuleBase::WARNING_QUIT("Input::Init", "Error during readin parameters.", 1); + return "Cannot open the INPUT file. Check the file path and read permissions.\n"; } ifs.clear(); @@ -379,12 +378,8 @@ std::string ReadInput::read_txt_input(Parameter& param, const std::string& filen if (ierr == 0) { - std::cout << " Error parameter list. " - << " The parameter list always starts with key word " - "'INPUT_PARAMETERS'. " - << std::endl; - ModuleBase::WARNING_QUIT("Input", - "Bad parameter, please check the input parameters in file INPUT", 1); + return "Missing INPUT_PARAMETERS header. The parameter list must start with INPUT_PARAMETERS.\n" + "See https://abacus.deepmodeling.com/en/latest/advanced/input_files/input-main.html\n"; } ascii_stream.rdstate(); @@ -406,8 +401,8 @@ std::string ReadInput::read_txt_input(Parameter& param, const std::string& filen } if (std::find(readvalue_items.begin(), readvalue_items.end(), p_item) != readvalue_items.end()) { - std::string warningstr = "The parameter " + p_item->label + " has been read twice."; - ModuleBase::WARNING_QUIT("ReadInput", warningstr); + return "The parameter " + p_item->label + " has been read twice. Specify it only once.\nSee " + + input_manual_link(p_item->label) + "\n"; } this->readvalue_items.push_back(p_item); // qianrui delete '/' 2024-07-10, because path has '/' head. @@ -417,9 +412,8 @@ std::string ReadInput::read_txt_input(Parameter& param, const std::string& filen { if (word[0] != '#' && word[0] != '/' && word[0] != '!') // if not recognized { - std::cout << " THE PARAMETER NAME '" << word << "' IS INCORRECT!" << std::endl; - ModuleBase::WARNING_QUIT("Input", - "Bad parameter, please check the input parameters in file INPUT", 1); + return "Unknown INPUT parameter '" + word + "'. Check the parameter name.\n" + "See https://abacus.deepmodeling.com/en/latest/advanced/input_files/input-main.html\n"; } // otherwise, it is a comment. However, ... // but it is not always to be shorter than 150 characters @@ -434,14 +428,11 @@ std::string ReadInput::read_txt_input(Parameter& param, const std::string& filen } else if (ascii_stream.bad()) { - ModuleBase::WARNING_QUIT("Input", - " Bad input parameters. ", 1); + return "Cannot read the INPUT parameter list: stream error.\n"; } else if (ascii_stream.fail()) { - ascii_stream.clear(); - ModuleBase::WARNING_QUIT("Input", - " fail to read parameters. ", 1); + return "Cannot read the INPUT parameter list: failed to extract a parameter.\n"; } } diff --git a/source/source_io/test/CMakeLists.txt b/source/source_io/test/CMakeLists.txt index a11fa9af8a..ef8609a0af 100644 --- a/source/source_io/test/CMakeLists.txt +++ b/source/source_io/test/CMakeLists.txt @@ -20,6 +20,27 @@ if(NOT ENABLE_MPI) return() endif() +add_executable(MODULE_IO_input_error_mpi test_read_input.cpp) +target_link_libraries(MODULE_IO_input_error_mpi PRIVATE parameter base device io_input abacus::linalg_libs) +if(ENABLE_COVERAGE) + add_coverage(MODULE_IO_input_error_mpi) +endif() +foreach(input_case IN ITEMS missing_file missing_header unknown_parameter duplicate_parameter + duplicate_string_eof invalid_value invalid_dependency valid) + add_test(NAME MODULE_IO_input_error_mpi_${input_case} + COMMAND ${CMAKE_COMMAND} + "-DREADER=$" + "-DMPIEXEC_EXECUTABLE=${MPIEXEC_EXECUTABLE}" + "-DMPIEXEC_NUMPROC_FLAG=${MPIEXEC_NUMPROC_FLAG}" + "-DMPIEXEC_PREFLAGS=${MPIEXEC_PREFLAGS}" + "-DMPIEXEC_POSTFLAGS=${MPIEXEC_POSTFLAGS}" + "-DINPUT_CASE=${input_case}" + "-DCASE_DIR=${CMAKE_CURRENT_BINARY_DIR}/input_error_${input_case}" + -P ${CMAKE_CURRENT_SOURCE_DIR}/test_read_input.cmake) + set_tests_properties(MODULE_IO_input_error_mpi_${input_case} + PROPERTIES PROCESSORS 4 TIMEOUT 45 ENVIRONMENT "OMP_NUM_THREADS=1") +endforeach() + add_test(NAME MODULE_IO_write_bands_parallel COMMAND ${MPIEXEC_EXECUTABLE} ${MPIEXEC_NUMPROC_FLAG} 4 ${MPIEXEC_PREFLAGS} $ ${MPIEXEC_POSTFLAGS} diff --git a/source/source_io/test/test_read_input.cmake b/source/source_io/test/test_read_input.cmake new file mode 100644 index 0000000000..f6e25d60c2 --- /dev/null +++ b/source/source_io/test/test_read_input.cmake @@ -0,0 +1,70 @@ +file(MAKE_DIRECTORY "${CASE_DIR}") +set(input_file "${CASE_DIR}/INPUT") +set(contents "INPUT_PARAMETERS\nbasis_type pw\nntype 1\n") +set(expected_exit 1) +set(expected_manual "input-main.html") + +if(INPUT_CASE STREQUAL "missing_file") + file(REMOVE "${input_file}") + set(expected "Cannot open the INPUT file") + set(expected_manual "") +elseif(INPUT_CASE STREQUAL "missing_header") + set(contents "basis_type pw\nntype 1\n") + set(expected "Missing INPUT_PARAMETERS header") +elseif(INPUT_CASE STREQUAL "unknown_parameter") + string(APPEND contents "unknown_input_option 1\n") + set(expected "Unknown INPUT parameter 'unknown_input_option'") +elseif(INPUT_CASE STREQUAL "duplicate_parameter") + string(APPEND contents "nspin 1\nnspin 2\n") + set(expected "The parameter nspin has been read twice") + set(expected_manual "input-main.html#nspin") +elseif(INPUT_CASE STREQUAL "duplicate_string_eof") + # Repeat a scalar string parameter with no value or final newline. + string(APPEND contents "basis_type") + set(expected "The parameter basis_type has been read twice") + set(expected_manual "input-main.html#basis-type") +elseif(INPUT_CASE STREQUAL "invalid_value") + string(APPEND contents "nspin 2abc\n") + set(expected "Expected one Integer from {1, 2, 4}") + set(expected_manual "input-main.html#nspin") +elseif(INPUT_CASE STREQUAL "invalid_dependency") + string(APPEND contents "nspin 2\nnoncolin true\n") + set(expected "Requires nspin==4") + set(expected_manual "input-main.html#nspin") +elseif(INPUT_CASE STREQUAL "valid") + string(APPEND contents "nspin 2\n") + set(expected_exit 0) +else() + message(FATAL_ERROR "Unknown INPUT test case: ${INPUT_CASE}") +endif() + +if(NOT INPUT_CASE STREQUAL "missing_file") + file(WRITE "${input_file}" "${contents}") +endif() + +execute_process( + COMMAND "${MPIEXEC_EXECUTABLE}" ${MPIEXEC_NUMPROC_FLAG} 4 ${MPIEXEC_PREFLAGS} + "${READER}" ${MPIEXEC_POSTFLAGS} "${input_file}" + WORKING_DIRECTORY "${CASE_DIR}" + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error + TIMEOUT 30) +string(APPEND output "${error}") + +if(NOT "${result}" STREQUAL "${expected_exit}") + message(FATAL_ERROR "Expected exit ${expected_exit}, got ${result}:\n${output}") +endif() +string(REGEX MATCHALL "INPUT reader process reached shutdown" exits "${output}") +list(LENGTH exits exit_count) +if(NOT exit_count EQUAL 4) + message(FATAL_ERROR "Only ${exit_count} of 4 processes reached reader shutdown:\n${output}") +endif() +if(expected_exit EQUAL 1) + foreach(diagnostic IN ITEMS "${input_file}" "${expected}" "${expected_manual}") + string(FIND "${output}" "${diagnostic}" found) + if(found EQUAL -1) + message(FATAL_ERROR "Missing diagnostic '${diagnostic}':\n${output}") + endif() + endforeach() +endif() diff --git a/source/source_io/test/test_read_input.cpp b/source/source_io/test/test_read_input.cpp new file mode 100644 index 0000000000..b2f3df03af --- /dev/null +++ b/source/source_io/test/test_read_input.cpp @@ -0,0 +1,47 @@ +#include "source_base/parallel_global.h" +#include "source_io/module_parameter/read_input.h" + +#include +#include +#include + +namespace +{ +void finish_reader() +{ + // Print before finalization so an early nonzero exit cannot make the + // MPI launcher terminate peers before their shutdown markers are written. + std::fputs("INPUT reader process reached shutdown\n", stderr); + std::fflush(stderr); + // Finalization makes a rank-0-only exit fail the test's timeout even if + // the MPI launcher would otherwise terminate the remaining ranks for us. + Parallel_Global::finalize_mpi(); +} +} // namespace + +int main(int argc, char** argv) +{ + if (argc != 2) + { + return 2; + } + int process_count = 1; + int thread_count = 1; + int rank = 0; + Parallel_Global::read_pal_param(argc, argv, process_count, thread_count, rank); + // Input-only tests do not create the legacy calculation communicators. + POOL_WORLD = MPI_COMM_NULL; + KP_WORLD = MPI_COMM_NULL; + INT_BGROUP = MPI_COMM_NULL; + BP_WORLD = MPI_COMM_NULL; + GRID_WORLD = MPI_COMM_NULL; + DIAG_WORLD = MPI_COMM_NULL; + std::atexit(finish_reader); + + const std::string filename = argv[1]; + ModuleIO::ReadInput reader(rank); + reader.check_ntype_flag = false; + Parameter input; + reader.read_parameters(input, filename); + return 0; +} diff --git a/source/source_io/test_serial/read_input_test.cpp b/source/source_io/test_serial/read_input_test.cpp index 059e52c462..e640708c72 100644 --- a/source/source_io/test_serial/read_input_test.cpp +++ b/source/source_io/test_serial/read_input_test.cpp @@ -238,7 +238,7 @@ TEST_F(InputTest, ValidateRelaxMethodScalar) expect_invalid_input("relax_invalid_INPUT", "relax_method " + invalid + "\n", invalid_method_message); } - expect_invalid_input("relax_new_removed_INPUT", "relax_new 1\n", "THE PARAMETER NAME 'relax_new' IS INCORRECT"); + expect_invalid_input("relax_new_removed_INPUT", "relax_new 1\n", "Unknown INPUT parameter 'relax_new'"); } TEST_F(InputTest, ValidateNoncollinearSpin)