Skip to content

delete_file_on_restart: false wrongly stops scans from resuming with the hdf5_v1 printer #485

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.

Sign up for GitHub

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

Open
anderkve wants to merge 5 commits into
base: master
Choose a base branch
from
Open

delete_file_on_restart: false wrongly stops scans from resuming with the hdf5_v1 printer #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db77e65
Trying to fix faulty if-else logic related resuming jobs with the hdf…
anderkve Feb 9, 2024
727542e
Moved comment in hdf5printer_v2.cpp
anderkve Feb 9, 2024
f561f72
Changed file suffix of expected combined hdf5 file, from '_combined_t…
anderkve Feb 9, 2024
fb18fab
Edited some error and log messages for clarity.
anderkve Feb 9, 2024
43b71a9
Merge branch 'master' into hdf5_v1_resume_fix
anderkve May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 55 additions & 48 deletions Printers/src/printers/hdf5printer/hdf5printer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ namespace Gambit

// Name of file where combined results from previous (unfinished) runs end up
std::ostringstream rename;
rename << finalfile << "_temp_combined";
rename << finalfile << "_combined";
Copy link
Collaborator

@ChrisJChang ChrisJChang Feb 27, 2024

Choose a reason for hiding this comment

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

This is probably the relevant part for my comment about restarting overwriting data.

tmp_comb_file = rename.str();

// HDF5 group (virtual "folder") inside output file in which to store datasets
Expand Down Expand Up @@ -420,50 +420,54 @@ namespace Gambit
#endif
if(HDF5::checkFileReadable(finalfile, msg_finalfile))
{
if(overwrite_file and not get_resume())
// Note: "not resume" means "start or restart"
if(not get_resume())
{
// Note: "not resume" means "start or restart"
// Delete existing output file
std::ostringstream command;
command << "rm -f "<<finalfile;
logger() << LogTags::printers << LogTags::info << "Running shell command: " << command.str() << EOM;
FILE* fp = popen(command.str().c_str(), "r");
if(fp==NULL)
{
// Error running popen
std::ostringstream errmsg;
errmsg << "rank "<<myRank<<": Error deleting existing output file (requested by 'delete_file_on_restart' printer option; target filename is "<<finalfile<<")! popen failed to run the command (command was '"<<command.str()<<"')";
printer_error().raise(LOCAL_INFO, errmsg.str());
}
else if(pclose(fp)!=0)
if(overwrite_file)
{
// Command returned exit code!=0, or pclose failed
std::ostringstream errmsg;
errmsg << "rank "<<myRank<<": Error deleting existing output file (requested by 'delete_file_on_restart' printer option; target filename is "<<finalfile<<")! Shell command failed to executed successfully, see stderr (command was '"<<command.str()<<"').";
printer_error().raise(LOCAL_INFO, errmsg.str());
// Delete existing output file
std::ostringstream command;
command << "rm -f "<<finalfile;
logger() << LogTags::printers << LogTags::info << "Running shell command: " << command.str() << EOM;
FILE* fp = popen(command.str().c_str(), "r");
if(fp==NULL)
{
// Error running popen
std::ostringstream errmsg;
errmsg << "rank "<<myRank<<": Error deleting existing output file (requested by 'delete_file_on_restart' printer option; target filename is "<<finalfile<<")! popen failed to run the command (command was '"<<command.str()<<"')";
printer_error().raise(LOCAL_INFO, errmsg.str());
}
else if(pclose(fp)!=0)
{
// Command returned exit code!=0, or pclose failed
std::ostringstream errmsg;
errmsg << "rank "<<myRank<<": Error deleting existing output file (requested by 'delete_file_on_restart' printer option; target filename is "<<finalfile<<")! Shell command failed to executed successfully, see stderr (command was '"<<command.str()<<"').";
printer_error().raise(LOCAL_INFO, errmsg.str());
}
}
}
else
{
// File exists, so check if 'group' is readable, and throw error if it exists
file_id = HDF5::openFile(finalfile);
std::string msg_group;
std::cout << "Group readable: " << finalfile << " , " << group << " : " << HDF5::checkGroupReadable(file_id, group, msg_group) << std::endl;
if(HDF5::checkGroupReadable(file_id, group, msg_group))
else
{
// Group already exists, error!
std::ostringstream errmsg;
errmsg << "Error preparing pre-existing output file '"<<finalfile<<"' for writing via hdf5printer! The requested output group '"<<group<<" already exists in this file! Please take one of the following actions:"<<std::endl;
errmsg << " 1. Choose a new group via the 'group' option in the Printer section of your input YAML file;"<<std::endl;
errmsg << " 2. Delete the existing group from '"<<finalfile<<"';"<<std::endl;
errmsg << " 3. Delete the existing output file, or set 'delete_file_on_restart: true' in your input YAML file to give GAMBIT permission to automatically delete it (applies when -r/--restart flag used);"<<std::endl;
errmsg << std::endl;
errmsg << "*** Note: This error most commonly occurs when you try to resume a scan that has already finished! ***" <<std::endl;
errmsg << std::endl;
printer_error().raise(LOCAL_INFO, errmsg.str());
// File exists, so check if 'group' is readable, and throw error if it exists
file_id = HDF5::openFile(finalfile);
std::string msg_group;
std::cout << "Group readable: " << finalfile << " , " << group << " : " << HDF5::checkGroupReadable(file_id, group, msg_group) << std::endl;
if(HDF5::checkGroupReadable(file_id, group, msg_group))
{
// Group already exists, error!
std::ostringstream errmsg;
errmsg << "Error preparing pre-existing output file '"<<finalfile<<"' for writing via hdf5printer! The requested output group '"<<group<<" already exists in this file! Please take one of the following actions:"<<std::endl;
errmsg << " 1. Choose a new group via the 'group' option in the Printer section of your input YAML file;"<<std::endl;
errmsg << " 2. Delete the existing group from '"<<finalfile<<"';"<<std::endl;
errmsg << " 3. Delete the existing output file, or set 'delete_file_on_restart: true' in your input YAML file to give GAMBIT permission to automatically delete it (applies when -r/--restart flag used);"<<std::endl;
errmsg << std::endl;
errmsg << "*** Note: This error most commonly occurs when you try to resume a scan that has already finished! ***" <<std::endl;
errmsg << std::endl;
printer_error().raise(LOCAL_INFO, errmsg.str());
}
HDF5::closeFile(file_id);
exit(1);
}
HDF5::closeFile(file_id);
exit(1);

}
}

Expand All @@ -490,7 +494,7 @@ namespace Gambit
{
// There is no combined output either, so disable resume mode
std::ostringstream msg;
msg<<"No temporary output from a previous scan found (or it is unreadable); this will be treated as a NEW scan";
msg<<"No combined output file " << tmp_comb_file << " from a previous scan found (or it is unreadable); this will be treated as a NEW scan";
std::cout<<msg.str()<<std::endl;
logger() << LogTags::info << msg.str();
set_resume(false);
Expand Down Expand Up @@ -533,7 +537,7 @@ namespace Gambit
logger() << LogTags::printers << LogTags::info << "Rank " << myRank << ": tmp_comb_file readable? " << HDF5::checkFileReadable(tmp_comb_file) << "(filename: " << tmp_comb_file << ")" << EOM;
if( HDF5::checkFileReadable(tmp_comb_file) )
{
logger() << LogTags::info << "Scanning existing temporary combined output file, to prepare for adding new data" << EOM;
logger() << LogTags::info << "Scanning existing temporary combined output file " << tmp_comb_file << ", to prepare for adding new data" << EOM;
// Open HDF5 file
file_id = HDF5::openFile(tmp_comb_file);

Expand Down Expand Up @@ -745,7 +749,7 @@ namespace Gambit
std::string msg;
if( HDF5::checkFileReadable(tmp_comb_file, msg) )
{
logger() << LogTags::repeat_to_cout << LogTags::info << "...Existing temporary combined output file was found and is readable" << EOM;
logger() << LogTags::repeat_to_cout << LogTags::info << "...Existing temporary combined output file " << tmp_comb_file << " was found and is readable" << EOM;
combined_file_readable=true;
}
else
Expand Down Expand Up @@ -821,7 +825,10 @@ namespace Gambit
else
{
std::ostringstream errmsg;
errmsg << " Process level temporary HDF5 output was detected, however the 'disable_combine_routines' option is set for the HDF5 printer plugin. The combine code is therefore not permitted to run, so this job cannot proceed. Please either manually combine the output files, restart the scan, or set this option to 'false'" << std::endl;
errmsg << " Process level temporary HDF5 output was detected, however the 'disable_combine_routines' option is set for the HDF5 printer plugin."
<< " The combine code is therefore not permitted to run, so this run cannot proceed. "
<< " To resume the previous run, either manually combine the temp files into a file " << tmp_comb_file << " and delete/move all the temp files,"
<< " or set the 'disable_combine_routines' option to 'false'. To instead start a *new* scan, run gambit with the '-r' flag." << std::endl;
printer_error().raise(LOCAL_INFO, errmsg.str());
}
}
Expand Down Expand Up @@ -1089,14 +1096,14 @@ namespace Gambit
// follow a fixed format and they all exist. We check for this before
// running this function, so this should be fine.

// If we set the second last flag 'true' then Greg's code will assume that a '_temp_combined' output file
// If we set the second last flag 'true' then Greg's code will assume that a '_combined' output file
// exists, and it will crash if it doesn't. So we need to first check if such a file exists.
bool combined_file_exists = Utils::file_exists(tmp_comb_file); // We already check this externally; pass in as flag?
logger() << LogTags::printers << LogTags::info << "combined_file_exists? " << combined_file_exists << EOM;
logger() << LogTags::printers << LogTags::info << "tmp_comb_file (" << tmp_comb_file << ") exists? " << combined_file_exists << EOM;
if(not combined_file_exists)
std::cout << "Combined file NOT found, combining now..." << std::endl;
std::cout << "Combined file " << tmp_comb_file << " NOT found, combining now..." << std::endl;
else
std::cout << "Combined file found, skipping combination" << std::endl;
std::cout << "Combined file " << tmp_comb_file << " found, skipping combination" << std::endl;
// Second last bool just tells the routine to delete the temporary files when it is done
// Last flag, if false, tells routines to throw an error if any expected temporary file cannot be opened for any reason
HDF5::combine_hdf5_files(tmp_comb_file, finalfile, group, metadata_group, num, combined_file_exists, true, false);
Expand Down
2 changes: 1 addition & 1 deletion Printers/src/printers/hdf5printer_v2/hdf5printer_v2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1480,9 +1480,9 @@ namespace Gambit
std::string msg_finalfile;
if(HDF5::checkFileReadable(file, msg_finalfile))
{
// Note: "not resume" means "start or restart"
if(not get_resume())
{
// Note: "not resume" means "start or restart"
if(overwrite_file)
{
// Delete existing output file
Expand Down
4 changes: 2 additions & 2 deletions Printers/standalone/manual_hdf5_combine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void usage()
"\n them from <filename>. Filenames are to be supplied in a single"
"\n ascii file, on separate lines."
"\n In this mode <filename> will only be used for naming the output"
"\n file (Note: WITHOUT the usual addition of '_temp_combined'!!!)."
"\n file (Note: WITHOUT the usual addition of '_combined'!!!)."
Copy link
Collaborator

@ChrisJChang ChrisJChang Feb 27, 2024

Choose a reason for hiding this comment

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

Does this mean that my naive restart test that doesn't include the _combined name is using the combine script incorrectly?

Copy link
Collaborator Author

@anderkve anderkve May 26, 2025

Choose a reason for hiding this comment

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

Is this actually the combine code that GAMBIT runs? Looks to me like this is just a standalone program -- and one we don't really use, since we typically use the Python script?

Copy link
Collaborator

@ChrisJChang ChrisJChang May 27, 2025

Choose a reason for hiding this comment

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

From the Core Meeting: This file never gets run, and can probably be deleted/moved to code storage.

"\n All files must have the same <group>."
"\n Note: Auxilliary ('RA') datasets will be IGNORED! These should"
"\n be combined during 'normal' combination of results from a single"
Expand Down Expand Up @@ -196,7 +196,7 @@ int main(int argc, char* argv[])

// Name of temporary combined file, if one exists
std::ostringstream name;
name << finalfile << "_temp_combined";
name << finalfile << "_combined";
tmp_comb_file = name.str();

combined_file_exists = Utils::file_exists(tmp_comb_file);
Expand Down
Loading