Repository: TREX-CoE/trexio (language: python)

Issue:
add 'r','w','a' options to trexio_open function (front end)



Discussion / hints:
'r' and 'w' is done in PR #14.
I don't see the difference between 'w' and 'a'.
'a' (append) would correspond to the case when a user wants to write in existing file, e.g. writing some missing blocks or overwriting existing ones. It protects from removing the existing file with data. Returns an error if the file does not exist and suggests to open with 'w' mode.
'w' would always create a new file to write in it. If the file exists, calling with this mode should return an error saying that file already exists and cannot be created, thus it has to be opened with 'a' mode.
What do you think?
OK. But we should also have a way to do 'a' or 'w'.
We can do
- 'a' : the file should exist
- 'w' : the file may or may not exist
- 'c' : the file should not exist (c: create)
- 'r' : the file should exist

Evgeny Posenitskiy writes:

> 'a' (append) would correspond to the case when a user wants to write in existing file, e.g. writing some missing blocks or overwriting existing ones. It protects from removing the existing file with data. Returns an error if the file does not exist and suggests to open with 'w' mode.
> 'w' would always create a new file to write in it. If the file exists, calling with this mode should return an error saying that file already exists and cannot be created, thus it has to be opened with 'a' mode.
> What do you think?


-- 
Anthony Scemama
IRSAMC / Laboratoire de Chimie et Physique Quantiques
UMR5626 CNRS/UPS
Toulouse


Fix patch:
diff --git a/src/trexio.org b/src/trexio.org
index 6363a851..050df1c2 100644
--- a/src/trexio.org
+++ b/src/trexio.org
@@ -164,7 +164,7 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b
   if (back_end <  0) return NULL;
   if (back_end >= TREXIO_INVALID_BACK_END) return NULL;
   
-  if (mode != 'r' && mode != 'w') return NULL;
+  if (mode != 'r' && mode != 'w' && mode != 'a') return NULL;
 
   trexio_t* result = NULL;
   
diff --git a/src/trexio_hdf5.org b/src/trexio_hdf5.org
index 06e8df25..97d2b919 100644
--- a/src/trexio_hdf5.org
+++ b/src/trexio_hdf5.org
@@ -159,26 +159,59 @@ trexio_exit_code trexio_hdf5_init(trexio_t* file) {
   trexio_hdf5_t* f = (trexio_hdf5_t*) file;
 
   /* If file doesn't exist, create it */
-  int f_ishere = 0;
+  int f_exists = 0;
   struct stat st;
   
-  if (stat(file->file_name, &st) == 0) {
-    printf("%s \n","HDF5 file already exists");
-   // RDWR OR RDONLY ???
-    f->file_id = H5Fopen(file->file_name, H5F_ACC_RDWR, H5P_DEFAULT);
-    f_ishere = 1;
+  if (stat(file->file_name, &st) == 0) f_exists = 1;
+  
+  if (f_exists == 1) {
+
+    switch (file->mode) {
+    case 'r': 
+      // reading the existing file -> open as RDONLY
+      f->file_id = H5Fopen(file->file_name, H5F_ACC_RDONLY, H5P_DEFAULT);
+      break;
+    case 'a': 
+      // appending the existing file -> open as RDWR
+      f->file_id = H5Fopen(file->file_name, H5F_ACC_RDWR, H5P_DEFAULT);
+      break;
+    case 'w': 
+      // writing the existing file -> overwrite it (_TRUNC) [_EXCL | H5F_ACC_DEBUG as an alternative]
+      f->file_id = H5Fcreate(file->file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
+      break;
+    }  
+
   } else {
-    f->file_id = H5Fcreate(file->file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
-    f_ishere = 0;
+
+    switch (file->mode) {
+    case 'r': 
+      // reading non-existing file -> error
+      return TREXIO_FAILURE;
+    case 'a': 
+    case 'w': 
+      // appending or writing non-existing file -> create it
+      f->file_id = H5Fcreate(file->file_name, H5F_ACC_EXCL, H5P_DEFAULT, H5P_DEFAULT);
+      break;
+    }  
+
   }  
 
-  /* Create groups in the hdf5 file */
-  if (f_ishere == 0){
-    f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
-    //f->electron_group = H5Gcreate(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
-  } else { 
-    f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT);
-    //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT);   
+  /* Create or open groups in the hdf5 file assuming that they exist if file exists */    
+  switch (file->mode) {
+    case 'r':
+    case 'a': 
+      if (f_exists == 1) {
+        f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT);
+      //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT);   
+      } else {
+        f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+      //f->electron_group = H5Gcreate(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+      }
+      break;
+    case 'w':
+      f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+      //f->electron_group = H5Gcreate(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+      break;
   }
   assert (f->nucleus_group > 0L);
   //assert (f->electron_group > 0L);
@@ -578,7 +611,6 @@ trexio_exit_code trexio_hdf5_write_nucleus_coord(const trexio_t* file, const dou
 }
 
     #+end_src
-** TODO Read/Write the charge attribute
 
 * File suffixes                                                     :noxport:
 


Test patch:
diff --git a/src/test.c b/src/test.c
index 32072827..c65e088e 100644
--- a/src/test.c
+++ b/src/test.c
@@ -59,6 +59,18 @@ int test_h5write() {
   rc = trexio_write_nucleus_num(file,25);
   assert (rc != TREXIO_SUCCESS);
 
+  // close current session
+  rc = trexio_close(file);
+  assert (rc == TREXIO_SUCCESS);
+
+  file = trexio_open(file_name, 'a', TREXIO_HDF5);
+  assert (file != NULL);
+  
+  // works: try to read the nucleus_num from existing file
+  rc = trexio_read_nucleus_num(file,&num);
+  assert (rc == TREXIO_SUCCESS);
+  assert (num == 12);
+
   // works: try to rewrite the nucleus_coord
   coord[0] = 666.666;
   rc = trexio_write_nucleus_coord(file,coord);
@@ -79,6 +91,7 @@ int test_h5read() {
   int64_t num;
   double* coord;
 
+  // test reading existing file [created by test_h5write()], should work
   file = trexio_open(file_name, 'r', TREXIO_HDF5);
   assert (file != NULL);
 
@@ -100,6 +113,23 @@ int test_h5read() {
   rc = trexio_close(file);
   assert (rc == TREXIO_SUCCESS);
 
+  // test reading non-existing file, should fail and return NULL
+  const char* file_name2 = "test_nonexisting.h5";
+  trexio_t* file2 = NULL;
+
+  file2 = trexio_open(file_name2, 'r', TREXIO_HDF5);
+  assert (file2 == NULL);
+
+  // test appending non-existing file, should create it
+  const char* file_name3 = "test_append.h5";
+  trexio_t* file3 = NULL;
+
+  file3 = trexio_open(file_name3, 'a', TREXIO_HDF5);
+  assert (file3 != NULL);
+
+  rc = trexio_close(file3);
+  assert (rc == TREXIO_SUCCESS);
+
   free(coord);
 
   return 0;