From f17e6b5c44f5e47d01c187b7dd860e6b09ceff72 Mon Sep 17 00:00:00 2001 From: wfg <wfg@pc0098504.ornl.gov> Date: Fri, 30 Sep 2016 17:01:29 -0400 Subject: [PATCH] 1) Working on Init functionality to read XML Config File 2) Added more info to CodingGuidelines 3) Added SMetadata.h file --- README.md | 5 ++ doc/CodingGuidelines | 134 +++++++++++++++++++++++++++++++++++++++---- include/CADIOS.h | 24 ++++---- include/SMetadata.h | 28 +++++++++ src/CADIOS.cpp | 51 +++++++++++++++- 5 files changed, 219 insertions(+), 23 deletions(-) create mode 100644 include/SMetadata.h diff --git a/README.md b/README.md index 32ad0c96b..5528fba19 100644 --- a/README.md +++ b/README.md @@ -1 +1,6 @@ # ADIOSPP + +Next generation ADIOS in C++ for exascale computations +Read ./doc/CodingGuidelines first + + diff --git a/doc/CodingGuidelines b/doc/CodingGuidelines index ce74b1099..3a03572fa 100644 --- a/doc/CodingGuidelines +++ b/doc/CodingGuidelines @@ -1,29 +1,141 @@ CodingGuidelines - Created on: Sep 30, 2016 - Author: wfg + Created on: Sep 30, 2016 + Rev: 0.1 + Author: William F Godoy, godoywf@ornl.gov -Introduce coding guidelines that all developers must follow as standard practice. +This document introduces coding guidelines that all developers must follow as standard practice. +This list is open as corrections and new ideas/suggestions come in place. Take them as mandatory to improve ADIOS. +Many items from this list are taken from Stroustrup, Sutter, and Meyers books. Objectives: 1) Improve collaboration: 1.a. Reduce new code development and integration times for developers and users, by making code easy to understand - 1.b. Allocate more time for discussion and pathfinding rather than understanding developers code + 1.b. Allocate more time for discussion and pathfinding rather than understanding developers' coding 1.c. Expand developers and users base -2) Execute new ideas faster +2) Execute new ideas faster 3) Allocate more time to research and publication from new ideas +4) Improve quality of final software product: reduce potential security risks (segmentation faults, core dumps, overflows) + and ease the integration of customers' products with ADIOS -C++ coding guidelines -Text -1) Use meaningful English words or well-known acronyms. +C++ coding guidelines, all mandatory in no particular order: + +Text style for readability (Coding format setup in Eclipse with Window > Preferences > C/C++ > CodeStyle > Formatters ) +1) Use meaningful English words (time not t, rank not r or rk), well-known acronyms (MPI, XML, CFD, GMRES, etc.) + or well-known short names (Config, Comm, 2D, 3D). + Examples: timeInitial instead of tIni, or work instead of wrk +2) Avoid "_" in names, adds unnecessary length (specially when mixed with STL containers) to the variable name and could conflict with name mangling. + Reserve it for prefix of special cases (see class members and lambda functions). + Use upper case letter instead: + Don't: std::vector< std::vector<double> > this_is_my_very_very_long_two_dimensional_vector_name + Do: std::vector< std::vector<double> > thisIsMyVeryVeryLongTwoDimensionalVectorName + +3) Use 4 spaces instead of tab. Tab behaves different in other editors (vi, emacs, gedit, slickedit, code blocks, etc.) +4) Use 4 spaces for nested logic structure indentation (like in Python), Eclipse has a CTRL+I option to set it automatically. + if( number1 < 1 ) + { + if( number2 < 2 ) + { + .... + } + } + + +5) Brackets: use an extra space for brackets + Do: + if( number < 1 ) + { + number = 4; + ... + } + + Don't: + if( number < 1 ){ + number = 4; ..... + .....} + + +6) Prefer the keyword "using" over "typedef". Only rename complex types, do not rename standard types (int, double, std::vector) + Don't: typedef std::vector<std::vector<double>> Vector2D; + Do: using std::vector<std::vector<double>> = Vector2D; + + +Documentation/Comments +1) Use Doxygen as the official API documentation tool. Eclipse has a template autocomplete option for function declarations. +2) Only use Doxygen in header declaration (.h) files, never in source definition (.cpp) +3) Use meaningful comments that are not redundant in the definition. Add context instead. + Example: Don't: int size; // define the size of the MPI processes + Do: int size; // global size for tracking purposes at close() -Classes / Structs -1) Classes will be initialized with a upper case "C" following by an upper, example: ( CADIOS, CNETCDF, CPHDF, C ) -2) Structs will be initialized with a upper case "S" and prefereed for plain old data, example: ( SVariables, S ) +Variable scope and Namespaces +1) Local scope variables and function arguments passed by value: first letter must be lower case ( int rank, int size ). +2) Functions: start with an upper case letter ( e.g. void Transform ( ), double Fourier( ), etc. ) +3) Lambda functions: use hungarian notation with prefix lf_ in the function name. ( e.g. auto lf_Fourier = []( const double argument ) ) +4) Avoid the "using namespace foo" clause, instead use the namespace explicitly. (e.g. std::cout not cout, adios::CHDF5 not CHDF5 ) + This prevents name conflict and allows identifying the function/variable source. +5) Declare variables right before they are needed, not at the beginning (Fortran style). + e.g. Don't ---> int rank; ...Many Lines of code... ; rank = SomeFunction( ); + Do---> ...Many Lines of code... ; int rank = SomeFunction( ); +6) Always pass by value primitives ( const int, double, const std::string ) if only a copy if needed. + For bigger objects and STL containers pass by reference always ( CNETCDF& netcdf, std::vector<std::string>& names ) +7) Use auto keyword only when it doesn't interfere with the logic understanding. Don't use it to replace primitives (int, unsigned int, double) + + +Classes / Structs +1) Classes will be initialized with a upper case "C" following by an upper, example: ( CADIOS, CNETCDF, CPHDF, CTransform, etc. ) +2) Structs will be initialized with a upper case "S". Reserve for plain old data, example: ( SVariables ) +3) Class/Struct member variables (not functions) will use hungarian notation ("m_" prefix) + followed by an upper case letter ( m_XMLConfig, m_GlobalSize ) +4) Only one file pair per class (class CADIOS in CADIOS.h and CADIOS.cpp), do not define several classes in the same file + + +Pointers and References +1) Avoid bare pointers (*p) at all cost for memory management (new/delete). Prefer smart pointers unique_ptr, shared_ptr. + Also, use smart pointers for polymorphism as objects are a dynamic (runtime) concept. +2) Prefer references (&) over pointers in passing by reference or for members of a class. + ONE Exception: Use a bare pointer only if a reference to an object can be null (NEVER use it for memory management). +3) Prefer the STL for memory management to minimize new structs/object (new/delete) creation + (e.g. std::vector<double> instead of double* ) +4) Avoid storing bare pointers in STL containers, they must be deallocated manually if so. + The STL is already taking care of object memory management. + Do: std::vector<double> + Don't: std::vector<double*> +5) Use reference and pointer identifiers attached to the type not the variable name, + Do: double* numbers , Don't: double * numbers + Do: std::vector<double>& vector, Don't: std::vector<double> &vector +6) Use RAII: resource allocation is initialization. + A good design should allocate memory resources at the constructor stage, or as soon as it is available. + Fine tuning memory should be used only if it causes a big impact. + +const / constexpr correctness, macro, and include statements +1) Always use const or constexpr when it is due as they are optimized and caught at compilation time +2) Do not use pre-processor macros (#define) unless it's a must (e.g. #ifdef __cplusplus or #pragma ) +3) Use const in functions that don't change the state of an Object or its members +4) Use const in function arguments (value or references) if the state of the argument is not changed. +5) Always use header (*.h) include guards, Eclipse does it automatically, using the file name only. + (e.g. in CADIOS.h start with #ifndef CADIOS_H_ + #define CADIOS_H_ ) +6) Only include header files at the beginning of the file (#include <cmath>). Make each *.h file self-contained. + Don't include the same header file in *.cpp already defined in an included *.h file. + Example: if cmath is included in CADIOS.h, which is included by CADIOS.cpp, then don't include cmath in CADIOS.cpp + + +Avoid mixing C-like equivalents +1) Use exclusive C++ header versions of their C-counterpart ( cmath over math.h, cstdlib over stdlib.h ). + Only use C-headers in these cases: + a. If there is no C++ equivalent (e.g. unistd.h ) + b. When C++ libraries are deprecated or not fully supported ( MPI, CUDA_C, PETSc ) + c. When interoperability is required with another language, C++ --> C ---> Fortran, or C++ --> C ---> JNI ---> Java +2) Do not use C functions if there is a C++ equivalent ( use std::cout instead of printf ) + + +C++ Exceptions +1) Use C++ standard exceptions for error handling. In rare cases a custom exception must be defined/declared. +2) Use informative messages for exception handling to complement the nature of the exception. \ No newline at end of file diff --git a/include/CADIOS.h b/include/CADIOS.h index 7774f3fda..6222bce38 100644 --- a/include/CADIOS.h +++ b/include/CADIOS.h @@ -12,19 +12,21 @@ #include <mpi.h> #include <memory> // unique_ptr - +#include "include/SMetadata.h" namespace adios { /** - * Parent abstract class CADIOS, Children must implement this class virtual functions. + * Class CADIOS, interface between user call and ADIOS library */ class CADIOS { -public: +public: // can be accessed by anyone - std::string m_XMLConfigFile; ///< XML File to be read containing configuration information + std::string m_XMLConfigFile; ///< XML File to be read containing configuration information + bool m_IsUsingMPI = false; ///< bool flag false: sequential, true: parallel MPI, to be checked instead of communicator + MPI_Comm* m_MPIComm = nullptr; ///< only used as reference to MPI communicator passed from parallel constructor, using pointer instead of reference as null is a possible value CADIOS( ); ///< empty constructor, to be defined. Since we have an empty constructor we can't have const variables not defined by this constructor @@ -37,20 +39,22 @@ public: /** * Parallel constructor for XML config file and MPI * @param xmlConfigFile passed to m_XMLConfigFile - * @param mpiCommunicator MPI communicator ...const to be discussed + * @param mpiComm MPI communicator ...const to be discussed */ CADIOS( const std::string xmlConfigFile, const MPI_Comm& mpiComm ); - virtual ~CADIOS( ); ///< virtual destructor overriden by children's own destructor + ~CADIOS( ); ///< virtual destructor overriden by children's own destructors - void Init( ); ///< calls to read XML file + void Init( ); ///< calls to read XML file among other initialization tasks +private: -protected: // only children can access this + SMetadata m_Metadata; ///< contains all metadata information from XML Config File - std::unique_ptr<MPI_Comm> m_MPIComm; ///< reference to MPI communicator passed from MPI constructor + void InitSerial( ); ///< called from Init, initialize Serial + void InitMPI( ); ///< called from Init, initialize parallel MPI - void ReadXMLConfigFile( ); ///< Must be implemented in CADIOS.cpp common to all classes + void ReadXMLConfigFile( ); ///< populates SMetadata by reading the XML Config File }; diff --git a/include/SMetadata.h b/include/SMetadata.h new file mode 100644 index 000000000..b4044cd4b --- /dev/null +++ b/include/SMetadata.h @@ -0,0 +1,28 @@ +/* + * SMetadata.h : contains metadata structs + * + * Created on: Sep 30, 2016 + * Author: wfg + */ + +#ifndef SMETADATA_H_ +#define SMETADATA_H_ + +namespace adios +{ +/** + * Metadata picked up from the XML config file, members can't be const as they are know after reading the config XML file + */ +struct SMetadata +{ + std::string hostLanguage; ///< Supported: C, C++, Fortran + + + + +}; + + +} + +#endif /* SMETADATA_H_ */ diff --git a/src/CADIOS.cpp b/src/CADIOS.cpp index 613eecac5..68734613a 100644 --- a/src/CADIOS.cpp +++ b/src/CADIOS.cpp @@ -6,13 +6,15 @@ * */ +#include <fstream> + #include "include/CADIOS.h" namespace adios { -//here assign default values +//here assign default values of non-primitives CADIOS::CADIOS( ) { } @@ -23,10 +25,55 @@ CADIOS::CADIOS( const std::string xmlConfigFile ): CADIOS::CADIOS( const std::string xmlConfigFile, const MPI_Comm& mpiComm ): - m_XMLConfigFile( xmlConfigFile ) + m_XMLConfigFile( xmlConfigFile ), + m_IsUsingMPI( true ), + m_MPIComm( mpiComm ) { } +void CADIOS::Init( ) +{ + if( m_IsUsingMPI == false ) + { + InitSerial( ); + } + else + { + InitMPI( ); + } +} + + +void CADIOS::InitSerial( ) +{ + ReadXMLConfigFile( ); +} + + +void CADIOS::InitMPI( ) +{ + +} + + +void CADIOS::ReadXMLConfigFile( ) +{ + std::ifstream xmlConfigStream( m_XMLConfigFile ); + + if( xmlConfigStream.good() == false ) //check file + { + const std::string errorMessage( "XML Config file " + m_XMLConfigFile + " could not be opened. " + "Check permissions or file existence\n"); + throw std::ios_base::failure( errorMessage ); + } + + + + +} + + + -- GitLab