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

Store output files in the given directory #406

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ChunYen-Chen
Copy link
Collaborator

Resolve #395.

@hyschive hyschive self-assigned this Jan 15, 2025
@hyschive hyschive added enhancement output Data output and log labels Jan 15, 2025
@hyschive
Copy link
Contributor

@ChunYen-Chen Please solve the conflicts.

Copy link
Contributor

@hyschive hyschive left a comment

Choose a reason for hiding this comment

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

@ChunYen-Chen Good job! I've added some minor comments.

@@ -77,6 +77,7 @@ int OPT__UM_IC_FLOAT8;
double COM_CEN_X, COM_CEN_Y, COM_CEN_Z, COM_MAX_R, COM_MIN_RHO, COM_TOLERR_R;
int COM_MAX_ITER;
double ANGMOM_ORIGIN_X, ANGMOM_ORIGIN_Y, ANGMOM_ORIGIN_Z;
char DUMP_DIR[MAX_STRING];
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename it to OUTPUT_DIR to match the names of other output-related variables

src/Auxiliary/Aux_CheckFileExist.cpp Outdated Show resolved Hide resolved
src/Auxiliary/Aux_CheckFileExist.cpp Outdated Show resolved Hide resolved
src/Auxiliary/Aux_CheckFileExist.cpp Outdated Show resolved Hide resolved
@@ -59,7 +59,8 @@ void CUAPI_DiagnoseDevice()


// record the device properties
const char FileName[] = "Record__Note";
char FileName[MAX_STRING];
sprintf( FileName, "%s/Record__Note", DUMP_DIR );
Copy link
Contributor

Choose a reason for hiding this comment

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

It triggers the following warning message. One simple solution could be reducing the size of DUMP_DIR (e.g., MAX_STRING-100), although this approach isn’t very elegant. Do you have a better solution?

GPU_API/CUAPI_DiagnoseDevice.cu:63:19: warning: ‘/Record__Note’ directive writing 13 bytes into a region of size between 1 and 512 [-Wformat-overflow=]
   63 |    sprintf( FileName, "%s/Record__Note", DUMP_DIR );

src/Auxiliary/Aux_Check_Parameter.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if Aux_CheckFolderExist() could also validate whether users have write permissions for the target folder (e.g., the folder might exist but not be owned by the user).

src/Output/Output_DumpData.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the line sprintf( FileName[NCOMP_FLUID+v], "%s_Passive%02d_%06d", Prefix, v, DumpID ); as well.

@hyschive
Copy link
Contributor

@ChunYen-Chen Additionally, should we also store OUTPUT_DIR in HDF5 snapshots and check it in Init_ByRestart_HDF5.cpp?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement output Data output and log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define a different directory to store output Data* files when running the simulation
2 participants