Skip to content

2026PKUCourseHW5:Fix file open check in utils.cpp#7136

Open
derekyjy wants to merge 2 commits intodeepmodeling:developfrom
derekyjy:hw5-fix-1
Open

2026PKUCourseHW5:Fix file open check in utils.cpp#7136
derekyjy wants to merge 2 commits intodeepmodeling:developfrom
derekyjy:hw5-fix-1

Conversation

@derekyjy
Copy link

Reminder

  • Have you linked an issue with this pull request?
  • Have you added adequate unit tests and/or case tests for your pull request?
  • Have you noticed possible changes of behavior below or in the linked issue?
  • Have you explained the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi? (ignore if not applicable)

Linked Issue

Fix #...

Unit Tests and/or Case Tests for my changes

  • A unit test is added for each new feature or bug fix.

What's changed?

  • Example: My changes might affect the performance of the application under certain conditions, and I have tested the impact on various scenarios...

Any changes of core modules? (ignore if not applicable)

  • Example: I have added a new virtual function in the esolver base class in order to ...

Copilot AI review requested due to automatic review settings March 25, 2026 14:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 after matrixFile.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.

Comment on lines +94 to +100
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);
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Comment on lines +97 to +100
if (!matrixFile.is_open()) {
std::cerr << "Error: failed to open file " << FileName << std::endl;
exit(EXIT_FAILURE);
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>.

Copilot uses AI. Check for mistakes.
Comment on lines +94 to 103
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
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
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.

2 participants