Add json parser for input files #5993

MFraters · 2018-03-04T02:34:07Z

This adds the possibility of parsing json files to the parameter handler. I tried to follow the xml implementation as closely as possible, but I think the xml implementation contains a bug, which doesn't to use spaces in the names of variables. That is solved here by making the mangler public and using it to escape/mangle the text.

If it is agreed that this is a correct implementation, I can add some tests. We also might want to make demangle public.

drwells · 2018-03-04T03:08:44Z

Thanks for taking the time to implement and submit this 👍

I have a few comments:

We should just make mangle and demangle internal functions that are defined only in parameter_handler.cc. If I understand this correctly we need it in the recursive parsing functions.
Where are the json and XML parsers different? I was not able to spot the difference.
Would it be possible to add a test that fails on master where we have an XML file with an invalid parameter name?

MFraters · 2018-03-04T12:31:55Z

Sure, I will move them. Yes that is correct, they are at least needed for the sjon parser, see point 3.
The only difference is the lines with read_xml and read_json and the mangle/key lines. But looking more closely, I see that the xml parser has some code and styling restrictions especially for xml, which are not needed for json. So I will simplify the code a bit.
The problem with the xml parser is a bit more fundamental. Calling it a bug might not have been entirely correct, since the description says it is only mend for data writen out by the parameter handler itself and the GUI, which is already mangled. The problem is that the xml standart doesn't support multi word keys, because a word after the space is considered to be an attribute. So that is the reason why the xml reader doesn't 'need' a mangler. We could still support multiword characters, because as far as I know we do not need attributes, but that is I think a different discussion.

MFraters · 2018-03-04T14:26:29Z

@drwells Thanks for the comments btw :)

Just a little clarification on 3: in short yes, that would just be an xml file with non mangled parameter names.

I have updated the code and I think that I have addressed your comments.

bangerth · 2018-03-05T02:16:40Z

include/deal.II/base/parameter_handler.h

@@ -963,6 +963,15 @@ class ParameterHandler : public Subscriptor
   */
  virtual void parse_input_from_xml (std::istream &input);

+  /**
+   * Parse input from an XML stream to populate known parameter fields. This


copy paste error here and below: not XML but JSON

bangerth · 2018-03-05T02:17:29Z

include/deal.II/base/parameter_handler.h

-   * Mangle a string so that it doesn't contain any special characters or
-   * spaces.
-   */
-  static std::string mangle (const std::string &s);


Yes, thanks for moving these. They were part of the class from back in a time when anonymous namespaces weren't available or didn't work well. They're obviously better placed in the .cc file.

bangerth · 2018-03-05T02:19:38Z

source/base/parameter_handler.cc

+  // declared in the ParameterHandler object); if so, copy the value of these
+  // nodes into the destination object
+  void
+  read_json_recursively (const boost::property_tree::ptree &source,


How does this function differ from the corresponding XML one? If I see this correctly, then the input is just the ptree, and it shouldn't matter whether we got that ptree by reading XML or reading JSON?

It doesn't differ yet. I am currently looking into supporting json arrays, and then it will differ quite a lot.

I was wrong, it differs a tiny bit. The json actually uses mangle on the key, where the xml one doesn't. I personally think the xml could also use the mangle, but I am sure the json one needs it. So should I keep them separate, or add the mangle to the xml file? And should I change the name then to xml_json or leave it at xml?

I'm actually pretty sure the XML also uses the mangle on the keys -- because that's how we internally store the data in ParameterHandler: the key is always mangled in the ptree data structure.

You can see that if you compare the tests/parameter_handler_read_xml.cc and tests/prm/parameter_handler_read_xml.prm files. Look, for example, for the "double 1" and "string list" parameters that have spaces in them.

So if you're mangling on the key, I wonder if you're really mangling twice.

The declarations in tests/parameter_handler/parameter_handler_read_xml.cc have spaces in them, but the xml file tests/parameter_handler/prm/parameter_handler_read_xml.prm which is read in doesn't (see for example double_201):

<?xml version="1.0" encoding="utf-8"?> <ParameterHandler><int1><value>2</value><default_value>1</default_value><documentation>doc 1</documentation><pattern>0</pattern><pattern_description>[Integer range -2147483648...2147483647 (inclusive)]</pattern_description></int1><int2><value>3</value><default_value>2</default_value><documentation>doc 2</documentation><pattern>1</pattern><pattern_description>[Integer range -2147483648...2147483647 (inclusive)]</pattern_description></int2><ss1><double_201><value>2.234</value><default_value>1.234</default_value><documentation>doc 3</documentation><pattern>2</pattern><pattern_description>[Double -1.79769e+308...1.79769e+308 (inclusive)]</pattern_description></double_201><ss2><double_202><value>5.321</value><default_value>4.321</default_value><documentation>doc 4</documentation><pattern>3</pattern><pattern_description>[Double -1.79769e+308...1.79769e+308 (inclusive)]</pattern_description></double_202></ss2></ss1><Testing_25testing><string_26list><value>__< & > ; /</value><default_value>< & > ; /</default_value><documentation>docs 1</documentation><pattern>4</pattern><pattern_description>[Anything]</pattern_description></string_26list><int_2aint><value>2</value><default_value>2</default_value><documentation/> <pattern>5</pattern><pattern_description>[Integer range -2147483648...2147483647 (inclusive)]</pattern_description></int_2aint><double_2bdouble><value>7.1415926</value><default_value>6.1415926</default_value><documentation>docs 3</documentation><pattern>6</pattern><pattern_description>[Double -1.79769e+308...1.79769e+308 (inclusive)]</pattern_description></double_2bdouble></Testing_25testing></ParameterHandler>

Yes, exactly: the original text that had the spaces has been mangled.

This is the original text which is read in. It is mangled because the xml writer outputs mangled text, because xml doesn't support multi word keys, and the xml ready seems to be written to only support xml which is writen out by the parameter handler.

So what I am trying to say is that the xml read function is only tested with already mangled keys. (I think because xml itself doesn't support unmangled keys)

Yes, that's correct -- the XML reader is written with XML files in mind that were written by the XML writer, and not by hand.

You can change that for the JSON though I would suggest that you keep the JSON reader and writer in sync so that you can read files back in that the JSON writer had previously output.

ah, I see what you mean. My preferred solution would be to always mangle and demangle like we do with the normal parameter output, but for now I will get it out and we can discuss this in a possible follow up patch.

masterleinad · 2018-03-06T00:09:31Z

/run-tests

bangerth · 2018-03-06T09:28:51Z

@MFraters -- we're big believers in an incremental approach. For the moment, can you remove the function I commented above and instead call the existing recursive function? Then add a test similar to the XML test? (Maybe also add a changelog entry.)

I think this is already some progress in functionality that's worth committing as is, and it should be relatively uncontroversial -- let's get that one in already.

In a second step, you can come back to the issue of how you want to represent JSON files that do not 1:1 map onto ParameterHandler objects, and in particular how you want to deal with arrays. That is a more controversial issue, and I think it would be useful to first open a github issue in which you can discuss how you want to approach this. We can then discuss this before you spend too much time on implementing the feature.

bangerth

Looks good with the exception of the two doc places. Can you fix those and let me know?

bangerth · 2018-03-12T04:17:27Z

include/deal.II/base/parameter_handler.h

+  /**
+   * Parse input from an JSON stream to populate known parameter fields. This
+   * could be from a file originally written by the print_parameters() function
+   * using the XML output style and then modified by hand as necessary, or from


bangerth · 2018-03-12T04:17:46Z

include/deal.II/base/parameter_handler.h

+   * could be from a file originally written by the print_parameters() function
+   * using the XML output style and then modified by hand as necessary, or from
+   * a file written using this method and then modified by the graphical
+   * parameter GUI (see the general documentation of this class).


Take out this sentence. The GUI doesn't use JSON (though it could, in principle).

bangerth · 2018-03-12T04:21:06Z

I just edited things myself. Let's let the tester run over this again, and if the tester is happy, anyone can commit!

bangerth · 2018-03-12T12:12:14Z

I forgot to ask for this: @MFraters, would you be willing to add an entry to doc/news/changes/minor as a follow-up pull request?

make mangle an internal function of ParameterHandler.

7167a17

MFraters force-pushed the add_json_parser_for_input_files branch 2 times, most recently from 22993e8 to 6a2197b Compare March 4, 2018 14:21

bangerth reviewed Mar 5, 2018

View reviewed changes

MFraters force-pushed the add_json_parser_for_input_files branch from 6a2197b to 3e0a6a8 Compare March 6, 2018 18:15

MFraters added 2 commits March 8, 2018 17:15

add json parser for input files.

6be5221

Add test for the read json parameter handler.

af10170

MFraters force-pushed the add_json_parser_for_input_files branch from 3e0a6a8 to af10170 Compare March 8, 2018 16:20

bangerth reviewed Mar 12, 2018

View reviewed changes

Fix a couple of places in the documentation of a function.

c5b9077

bangerth added the Reviewed and ready to merge label Mar 12, 2018

masterleinad merged commit 06b5abd into dealii:master Mar 12, 2018

This was referenced Mar 12, 2018

World builder structure geodynamics/aspect#2122

Closed

add change entry for the add_json_parser_for_input_files pull request. #6042

Merged

marcfehling mentioned this pull request Dec 9, 2020

ParameterHandler::print_parameters() -- No demangle for XML and JSON #11342

Closed

marcfehling mentioned this pull request Jan 10, 2024

ParameterHandler: *.json files with keys containing underscores cannot be parsed #16453

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add json parser for input files #5993

Add json parser for input files #5993

MFraters commented Mar 4, 2018

drwells commented Mar 4, 2018

MFraters commented Mar 4, 2018

MFraters commented Mar 4, 2018

bangerth Mar 5, 2018

bangerth Mar 5, 2018

bangerth Mar 5, 2018

MFraters Mar 6, 2018

MFraters Mar 6, 2018

bangerth Mar 8, 2018

MFraters Mar 8, 2018

bangerth Mar 8, 2018

MFraters Mar 8, 2018

MFraters Mar 8, 2018 •

edited

bangerth Mar 8, 2018

MFraters Mar 8, 2018

masterleinad commented Mar 6, 2018

bangerth commented Mar 6, 2018

bangerth left a comment

bangerth Mar 12, 2018

bangerth Mar 12, 2018

bangerth commented Mar 12, 2018

bangerth commented Mar 12, 2018

Add json parser for input files #5993

Add json parser for input files #5993

Conversation

MFraters commented Mar 4, 2018

drwells commented Mar 4, 2018

MFraters commented Mar 4, 2018

MFraters commented Mar 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MFraters Mar 8, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masterleinad commented Mar 6, 2018

bangerth commented Mar 6, 2018

bangerth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bangerth commented Mar 12, 2018

bangerth commented Mar 12, 2018

MFraters Mar 8, 2018 •

edited