From d8e30aaf3f3e676e0b772db4a4a7bab3ba4ce206 Mon Sep 17 00:00:00 2001 From: Mark Dewing Date: Wed, 7 Jun 2017 11:55:12 -0500 Subject: [PATCH 1/2] Initialize is_open value in ReadFileBuffer. Address comments in issue #253. - initialize the value of is_open to false by default - Update the filename in the APP_ABORT messages - change get_file_length to be a member function. --- src/QMCHamiltonians/ECPComponentBuilder.cpp | 16 ++++++++-------- src/QMCHamiltonians/ECPComponentBuilder.h | 4 +++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/QMCHamiltonians/ECPComponentBuilder.cpp b/src/QMCHamiltonians/ECPComponentBuilder.cpp index 6fac883da7..42898b381a 100644 --- a/src/QMCHamiltonians/ECPComponentBuilder.cpp +++ b/src/QMCHamiltonians/ECPComponentBuilder.cpp @@ -54,12 +54,12 @@ bool ECPComponentBuilder::parse(const std::string& fname, xmlNodePtr cur) return read_pp_file(fname); } -int get_file_length(std::ifstream *fin) +int ReadFileBuffer::get_file_length(std::ifstream *f) { - fin->seekg (0, std::ios::end); - int length = fin->tellg(); - fin->seekg (0, std::ios::beg); - return length; + f->seekg (0, std::ios::end); + int len = f->tellg(); + f->seekg (0, std::ios::beg); + return len; } bool ReadFileBuffer::open_file(const std::string &fname) @@ -104,13 +104,13 @@ bool ECPComponentBuilder::read_pp_file(const std::string &fname) bool okay = buf.open_file(fname); if(!okay) { - APP_ABORT("ECPComponentBuilder::parse Missing PP file " + fname +"\n"); + APP_ABORT("ECPComponentBuilder::read_pp_file Missing PP file " + fname +"\n"); } okay = buf.read_contents(); if(!okay) { - APP_ABORT("ECPComponentBuilder::parse Unable to read PP file " + fname +"\n"); + APP_ABORT("ECPComponentBuilder::read_pp_file Unable to read PP file " + fname +"\n"); } xmlDocPtr m_doc = xmlReadMemory(buf.contents(),buf.length,NULL,NULL,0); @@ -118,7 +118,7 @@ bool ECPComponentBuilder::read_pp_file(const std::string &fname) if (m_doc == NULL) { xmlFreeDoc(m_doc); - APP_ABORT("ECPComponentBuilder::parse xml file "+fname+" is invalid"); + APP_ABORT("ECPComponentBuilder::read_pp_file xml file "+fname+" is invalid"); } // Check the document is of the right kind xmlNodePtr cur = xmlDocGetRootElement(m_doc); diff --git a/src/QMCHamiltonians/ECPComponentBuilder.h b/src/QMCHamiltonians/ECPComponentBuilder.h index b633550f4a..60573c0a46 100644 --- a/src/QMCHamiltonians/ECPComponentBuilder.h +++ b/src/QMCHamiltonians/ECPComponentBuilder.h @@ -86,10 +86,12 @@ class ReadFileBuffer char *cbuffer; std::ifstream *fin; Communicate *myComm; + int get_file_length(std::ifstream *f); + public: bool is_open; int length; - ReadFileBuffer(Communicate *c) : cbuffer(NULL), fin(NULL), myComm(c), length(0) {} + ReadFileBuffer(Communicate *c) : cbuffer(NULL), fin(NULL), myComm(c), is_open(false), length(0) {} bool open_file(const std::string &fname); bool read_contents(); char *contents() { return cbuffer; } From 5ae4be2d0582f8e7652195ed98b3b86bbbed24be Mon Sep 17 00:00:00 2001 From: Mark Dewing Date: Wed, 7 Jun 2017 13:04:32 -0500 Subject: [PATCH 2/2] Allow reuse of ReadFileBuffer. If a file has been opened using ReadFileBuffer, close it and clear the buffer before opening the new file. Add a unit test for the re-open case. Make get_file_length const, as it does not modify the class. --- src/QMCHamiltonians/ECPComponentBuilder.cpp | 21 ++++++++++++++++++++- src/QMCHamiltonians/ECPComponentBuilder.h | 3 ++- src/QMCHamiltonians/tests/test_ecp.cpp | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/QMCHamiltonians/ECPComponentBuilder.cpp b/src/QMCHamiltonians/ECPComponentBuilder.cpp index 42898b381a..6f303ef17b 100644 --- a/src/QMCHamiltonians/ECPComponentBuilder.cpp +++ b/src/QMCHamiltonians/ECPComponentBuilder.cpp @@ -54,7 +54,7 @@ bool ECPComponentBuilder::parse(const std::string& fname, xmlNodePtr cur) return read_pp_file(fname); } -int ReadFileBuffer::get_file_length(std::ifstream *f) +int ReadFileBuffer::get_file_length(std::ifstream *f) const { f->seekg (0, std::ios::end); int len = f->tellg(); @@ -62,8 +62,27 @@ int ReadFileBuffer::get_file_length(std::ifstream *f) return len; } +void ReadFileBuffer::reset() +{ + if (is_open) + { + if(myComm == NULL || myComm->rank() == 0) + { + delete fin; + fin = NULL; + } + delete[] cbuffer; + cbuffer = NULL; + is_open = false; + length = 0; + } +} + bool ReadFileBuffer::open_file(const std::string &fname) { + + reset(); + if (myComm == NULL || myComm->rank() == 0) { fin = new std::ifstream(fname.c_str()); diff --git a/src/QMCHamiltonians/ECPComponentBuilder.h b/src/QMCHamiltonians/ECPComponentBuilder.h index 60573c0a46..7041e61a97 100644 --- a/src/QMCHamiltonians/ECPComponentBuilder.h +++ b/src/QMCHamiltonians/ECPComponentBuilder.h @@ -86,7 +86,7 @@ class ReadFileBuffer char *cbuffer; std::ifstream *fin; Communicate *myComm; - int get_file_length(std::ifstream *f); + int get_file_length(std::ifstream *f) const; public: bool is_open; @@ -95,6 +95,7 @@ class ReadFileBuffer bool open_file(const std::string &fname); bool read_contents(); char *contents() { return cbuffer; } + void reset(); ~ReadFileBuffer() { delete[] cbuffer; diff --git a/src/QMCHamiltonians/tests/test_ecp.cpp b/src/QMCHamiltonians/tests/test_ecp.cpp index 83131084d2..cf1fbc8a52 100644 --- a/src/QMCHamiltonians/tests/test_ecp.cpp +++ b/src/QMCHamiltonians/tests/test_ecp.cpp @@ -96,5 +96,26 @@ TEST_CASE("ReadFileBuffer_ecp","[hamiltonian]") // TODO: add more checks that pseudopotential file was read correctly } +TEST_CASE("ReadFileBuffer_reopen","[hamiltonian]") +{ + // Initializing with no Communicate pointer under MPI, + // this will read the file on every node. Should be okay + // for testing purposes. + ReadFileBuffer buf(NULL); + bool open_okay = buf.open_file("simple.txt"); + REQUIRE(open_okay == true); + + bool read_okay = buf.read_contents(); + REQUIRE(read_okay); + REQUIRE(buf.length == 14); + + open_okay = buf.open_file("C.BFD.xml"); + REQUIRE(open_okay == true); + + read_okay = buf.read_contents(); + REQUIRE(read_okay); + REQUIRE(buf.length > 14); +} + }