2026PKUCourseHW5:Fix file open check in utils.cpp#7136
2026PKUCourseHW5:Fix file open check in utils.cpp#7136derekyjy wants to merge 2 commits intodeepmodeling:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make loadMatrix in the Genelpa utilities fail fast when the input matrix file cannot be opened, instead of proceeding with an invalid stream.
Changes:
- Added an
is_open()check aftermatrixFile.open(FileName)(root rank only). - Printed an error message and terminated on file-open failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (myid == ROOT_PROC){ | ||
| matrixFile.open(FileName); | ||
|
|
||
| double* b; // buffer | ||
| if (!matrixFile.is_open()) { | ||
| std::cerr << "Error: failed to open file " << FileName << std::endl; | ||
| exit(EXIT_FAILURE); | ||
| } |
There was a problem hiding this comment.
In an MPI/BLACS run, calling exit(EXIT_FAILURE) only on ROOT_PROC will leave the other ranks continuing into Cpdgemr2d(...), which can hang/crash because a collective is entered with one rank missing. Consider broadcasting the is_open status to all ranks and having all ranks take the same error path, or aborting the whole BLACS/MPI communicator (e.g., via MPI_Abort on the communicator associated with blacs_ctxt).
| if (!matrixFile.is_open()) { | ||
| std::cerr << "Error: failed to open file " << FileName << std::endl; | ||
| exit(EXIT_FAILURE); | ||
| } |
There was a problem hiding this comment.
exit/EXIT_FAILURE are used here without including <cstdlib> (or <stdlib.h>). This can fail to compile depending on transitive includes and toolchain settings; please add the proper include and prefer std::exit(EXIT_FAILURE) if using <cstdlib>.
| if (myid == ROOT_PROC){ | ||
| matrixFile.open(FileName); | ||
|
|
||
| double* b; // buffer | ||
| if (!matrixFile.is_open()) { | ||
| std::cerr << "Error: failed to open file " << FileName << std::endl; | ||
| exit(EXIT_FAILURE); | ||
| } | ||
| } | ||
| double* b; // buffer | ||
| const int MAX_BUFFER_SIZE = 1e9; // max buffer size is 1GB |
There was a problem hiding this comment.
The new block’s formatting deviates from the surrounding style (brace placement, mixed tabs/spaces, and double* b; losing indentation). Please reformat to match the file’s existing indentation and brace conventions to keep diffs clean and avoid accidental whitespace issues.
Reminder
Linked Issue
Fix #...
Unit Tests and/or Case Tests for my changes
What's changed?
Any changes of core modules? (ignore if not applicable)