search

QMCPACK / qmcpack

Main repository for QMCPACK, an open-source production level many-body ab initio Quantum Monte Carlo code for computing the electronic structure of atoms, molecules, and solids with full performance portable GPU support
http://www.qmcpack.org
Other
297 stars 139 forks source link

consolidate XML input/output wrapping and documentation needed #1813

Open ye-luo opened 5 years ago

ye-luo commented 5 years ago

libxml2 is a C library and easy to leak memory if used improperly. See #1801 To avoid these problems, xml usage needs to be cleanly wrapped and we are doing so. The other purpose of wrapping is to do validation at the wrapper level.

Our wrapper classes live in src/OhmmsData XMLParsingString.h handles xml node content to string and one attribute to string. ParameterSet.h handles a set of parameters like

  <qmc>
    <parameter name="walkers">                1 </parameter>
    <parameter name="blocks">                 2 </parameter>
    <parameter name="timestep">             1.0 </parameter>
    <parameter name="usedrift">              no </parameter>
  </qmc>

AttributeSet.h handles a set of attributes like

<determinantset type="einspline" href="pwscf.pwscf.h5" tilematrix="1 0 0 0 1 0 0 0 1" 
 twistnum="0" source="ion" meshfactor="1.0" precision="float" size="4"/>

By grepping the source with xmlGet/xmlSet/xmlNew, I still find many direct use of xml outside the wrapper class. I think we can make better use of our wrapper class and kill all the ugly casting in

https://github.com/QMCPACK/qmcpack/blob/1bc600726713c6758237bb11dbb91837110077b2/src/QMCTools/QMCGaussianParserBase.cpp#L1073

Also in QMCCostFunctionBase::updateXmlNodes we need the wrapper class to handle writing xml instead of doing it directly.

jtkrogel commented 5 years ago

How are you handling the read of/iteration over child XML elements for validation?

prckent commented 5 years ago

Another way of saying "make better use of our wrapper class" would be that we should properly abstract xml handling and not call libxml2 directly from anywhere other than the wrapper. This makes for any easy check on where to focus - any file importing or calling libxml2 other than the wrapper needs fixing.

jtkrogel commented 5 years ago

It would be nice down the road to handle a each XML element with a single class instead of one class for attributes, another class for parameters, yet another for text, and (currently) no class for child elements.

jtkrogel commented 5 years ago

(Removed) See discussion for this date in #407.

prckent commented 5 years ago

Long term the current input parsing needs to be totally rethought and redone. Here we do triage only i.e. use libxml2 correctly.

jtkrogel commented 5 years ago

Agreed. I just wanted to keep the rethinking part going. I will move my comment to #407.