Introduced a "type guesser", eventually for use by the HDF5 writer. #74
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some files aren't finished yet, and will be uploaded in a few days. So, this particular update won't build!! Added broad infrastructure for HDF5 handling... ...Wrapper class ...Assignment operators ...convert() functions ...Constructors ...read() functions ...write() functions Etc. Completed many (not all) HDF5 capabilities. Very basic HDF5 write needs to be smarter (not string-only). HDF5 reading still needs work. Touchups are needed here and there throughout the new HDF5 code. In particular, we need smart, non-string handling. Small tweaks were made to XML and JSON material, for consistency. Some comment changes and other odds and ends.
To be used for non-GNDS products, e.g. NDI3. When ProjectDir is given, namespaces and various other constructs are done differently. Made miscellaneous small improvements in various other files. For example: direct read() and write() in Component, aligning with Node's. So: autogenerated classes have those read()s and write(); there's no need for a user to explicitly go through Node.
Won't compile right now, but wanted to get this in.
For [optional] vector child-node fields, there's a setter that takes an element to push_back. If the optional has no value, it's given one first. Experience showed that something like this would be helpful. Updated code generator accordingly. Also, combined the default and "from fields" constructors in generated classes. This makes the generated classes shorter, and also, based on our experience with a prototype data format, proves to make object construction simpler. A generated class might have, for instance, a few metadata, followed by one or more vector or optional-vector child nodes. It's convenient for someone to initialize the object with metadata, and then to add vector elements individually later. This change helps make this process simpler and shorter. I also implemented another feature that experience with a prototype autogenerated set of classes suggested would be helpful. Before, getters with index or label looked for *metadata* named "index" or "label". Now, if no metadatum called "index" exists in the type of the vector's element (or any of its alternatives, if it's a variant), then index is interpreted to be a regular vector index, as in the "[]" operator. Note that someone who uses the code generator must be aware that the interpretation of (index) getters therefore depends fundamentally on whether or not the relevant vector element type has, or doesn't have, an "index" metadatum. This change means we essentially get an additional feature for free, as (index) previously would just fail unless an "index" metadatum were found. Removed a "GNDS"-centric comment in Component prettyprinted output. The comment probably wasn't particularly informative anyway. Updated test codes to reflect this change.
Regenerated the prototype codes. The changes to these also reflect other work I'd done recently on the code generator, as they hadn't been regenerated for a while.
That is to say, classes that have no metadata or child nodes, but still have data, e.g. with <foo>1.2 3.4 5.6</foo>. OCD'd a cmake-related file. Some more work regarding HDF5 format. Still not entirely complete. I think ".hdf"/".HDF" aren't used as HDF5 file extensions. They need a '5'. Made some diagnostic-message terminology more consistent.
Isn't very smart for output; just keeps everything as strings. That will be remedied at a later time. Was tested for functionality on entire set of GNDS v1.9 files. All converted. ...original total XML size: 2.9G ...new total HDF5 size: 21G So, a little over a factor of 7 larger. :-/ Miscellaneous other code changes were put in place during this work. Made a few diagnostic messages, and other minor constructs here and there, more consistent with one another. Added a convert() that proved to be needed for a certain disambiguation. This was noticed while working on some HDF5 input capabilities.
The former was taken from GNDS, but the latter is much more descriptive.
This is the first of a few simplifications I've been planning to do with the code base. Having `Child<>` objects contain a flag, indicating "is a node of this name allowable as a top-level GNDS node," originally seemed like a good idea. With our later decision to autogenerate GNDS Standard Interfaces from GNDS specs, having such a capability in the Core Interface really isn't necessary. The Core Interface can be safely generic, uncluttered with the capability of checking such a thing. Users will prefer our GNDS Standard Interfaces, which, having been generated from GNDS specifications, will be designed so that GNDS hierarchies will have the correct structure. Our Code Generator is also being used to design another (non-GNDS) library, and more such uses may follow. Checking if a root node has a valid *GNDS* name clearly wouldn't be wanted for other libraries. For this reason, we'd already switched off the check by default. At this point, however, we don't think that having even the ability to check such a thing, in the Core Interface, is worth the extra clutter that it creates in the code base.
The removed material was for a very old capability that was intended to help users make Meta<> and Child<> objects.
I hadn't given it much in the way of capabilities yet, but then it was all superseded by the ability to easily manipulate `Meta<>` and `Child<>` objects, e.g. with constructs such as `int{}/Meta<>(...)` to change the type associated with the `Meta<>` object.
The old keyword builder stuff never got much use, didn't have much to offer, and simply isn't needed any longer -- better capabilities exist, and in fact have existed for a long time. I just hadn't gotten around to removing the old code.
Removing it means a bit less code, one less executable to build in the test suite, and nothing really lost.
Gets rid of the annoying compiler warning about `tmpnam()`. The new formulation required a number of changes in the logic here and there.
Made certain HDF5-related operations more efficient.
A while back, I renamed class BodyText to class BlockData. It turns out that the "body [text]" terminology still appeared here and there. This PR basically completes the terminology change. I renamed relevant constructs, and removed "body" terminology at least in regards to block data.
A class generated by our code generator currently looks something like this:
```
class Foo : public Component {
// Some constructs to help with the Component base
// ...
struct {
// Objects for metadata and child nodes
} content;
// ...
// Getters for the objects in struct content
// ...
// Setters for the objects in struct content
// ...
// Constructors, etc.
};
```
For upcoming work, I'm considering a fundamental change to the above layout with respect to data, getters, and setters. The change would, I believe, make it easier to substantially enhance the capabilities offered by generated classes.
The change - if it ultimately happens - involves, among other things, getting rid of `struct content`.
In anticipation of this possible change, then, I went through the code base and tried to reduce the use of expressions like `obj.content.foo`, which assumes the existence of a data member `foo` in something called `content`. Even with the current structure of the generated classes, `obj.content.foo` can be replaced by `obj.foo()`, i.e. the getter than gets, well, `obj.content.foo`. With the possible future work, the getter (or something that replaces it) will do the moral equivalent to what it does now, but `struct content` won't be there. So, better to write `obj.foo()` where we previously wrote `obj.content.foo`.
So, the purpose of the changes in this commit can be summarized as follows: (1) They don't change the meaning of the current code. (2) They help set the stage for the possible future more-substantial changes to the generated code, by reducing the reliance one `content.something` terminology all over the place.
The changes, while mostly simple and easy to understand, triggered some SFINAE issues that were tricky to track down. New comments in BlockData's `detail.hpp` function explain it, if anyone cares.
Also in this PR...
General changes:
- Some minor updates to SFINAE, on top of those discussed above.
- Simplified a few things in the code generator.
- Code generator updates reflect goal of using "content." less often.
- Ditto for a relevant custom.hpp file.
Also, consistent with the goals described above, I replaced the various uses of "content." in several test codes, by writing getters (where they weren't there already) and calling those instead.
This is a change I've wanted to do for a while. I think it's the right thing to do. See the largest change in ``primer.rst`` for a brief description of why I did this. The Core Interface's internal structure has always made use of a handful of special names, to identify certain nodes that aren't in GNDS proper, but which need to be used in order to store certain special content. Naturally, special content needs special handling. Example: what arrives through PugiXML as XML CDATA content *was* stored in GNDStk's internal structure under a child node called ``cdata``. In fact, it was in a ``text`` metadatum inside of a ``cdata`` node. Now, the node is called ``#cdata`` and the metadatum is called ``#text``. That is, each name is now prefixed with ``#``. Adding the ``#`` prefix is just a small change, but it's meant for a couple of important purposes. First, its presence allows someone who looks at GNDStk's code base to easily identify special nodes -- which, generally, need special handling, at least in regards to activities such as I/O. Second, it's always possible that a future GNDS standard (or, importantly, another format that someone might design with our code generator) might want to have a field called ``foo``, where foo is one of our handful of special names that trigger special handling. By renaming ours to ``#foo`` instead, no conflicts will arise. (And someone won't use the ``#`` themselves for the name of a field. It wouldn't work when the associated class or object is created in C++, which of course doesn't allow such a character in variable names.) I'd have liked to use ``$`` in place of ``#``, but realized right away that doing so would be problematic. Functions like ``Node.one()`` use regular-expression searches, and the ``$`` character has a special meaning in C++ regular expressions. Clearly, then, using ``$`` would lead to various problems. So, we chose what we believe is a reasonable alternative, and one that we think is as good as any other. Note that the changes in this PR, while relatively simple, were a bit more involved than just changing ``foo`` to ``#foo`` for every ``special`` name ``foo``. Where JSON is involved, for example, you'll see that the use of ``#`` sometimes necessitated changes in node order, given that the JSON library orders nodes lexicographically. Also, some strings for special node names, for example ``xml``, also have meaning for other things: ``xml`` was a special node name, but ``xml`` is also a string someone can use, in certain places, to say that they want XML format. So, some instances of the string ``xml`` are now ``#xml``, but others remain as ``xml``.
For quite some time, GNDStk's Core Interface had a large number of `Meta` and `Child` objects in its `basic::` and `misc::` namespaces. This was set up prior to us making our current plan: to have GNDStk Standard Interfaces for GNDS 2.0, etc. Going forward, we anticipate that each Standard Interface will have all necessary `Meta` and `Child` objects built for it automatically, by our code generator, when our code generator generates the Standard Interface's classes. That means we need to keep only the current `Meta` and `Child` objects that happen to be used in existing test codes. Or, in a few instances, elsewhere in the Core Interface. Future work (not included right now) will consist of phasing out any remaining uses of these objects in Core Interface code. (Where that happens, we'll just make `Meta` or `Child` objects directly where they're needed.) Then, finally, we'll move any remaining `Meta` and `Child` objects - ones that are used in the test suite - into non-Core-Interface files that would be included only in test-suite code.
I implemented a useful feature that was requested by a user. When using Component's prettyprinter, previously block data were *all* printed. For example, if a block of data had 1000 elements, then all 1000 elements were printed by the prettyprinter. Now, we provide a user-settable variable, `GNDStk::truncate`. A negative value for this variable means: print all block-data elements, just like we did before. The default is `-1`, so printing all elements is the default. If `GNDStk::truncate` is `0` or positive, it means: print at most that number of elements. Whenever fewer elements are printed than exist, a comment to that effect is emitted. The comments mentions how many values actually exist, as that information may be important to someone to know. I also updated BlockData's `write.test.cpp` so that it tests the new feature, and I made some miscellaneous cosmetic changes to that file as well. Finally (and this is related to Component's prettyprinting, so this PR seemed like a good place to do it), I removed the `GNDStk::across` alias to `GNDStk::columns`. I'd had it, previously, because there was a `columns` in another namespace, and I wanted to provide something that worked even in the face of multiple `using` directives. The other `columns` disappeared, however, in an earlier PR in which we got rid of most `Meta` and `Child` objects.
We're generally trying to limit source-code lines to 80 characters or less in length. This isn't an absolute requirement, but, in keeping with the idea, I reformatted longer lines here and there in the hpp and cpp files throughout the code base, where doing so made sense. Sometimes, such codes have string literals (as with `R"..."`) with lines of more than 80 characters. Those I left as-is; the lines in question are supposed to be precisely the way they are. Also, for now, I didn't reformat long lines in other types of files. That includes, for example, various `CMakeLists.txt` files throughout GNDStk. Building very slightly on an earlier PR, I also made a small change to the `GNDStk::columns` variable, so that it would be more consistent, in terms of usage, with the related `GNDStk::truncate` variable. This has nothing to do with 80+ character lines, but was a simple change, so I decided to include it here. As usual, I modified a few comments and made small cosmetic changes in a few places.
Earlier during the development of GNDStk, we examined existing GNDS files and accounted for all of the node and metadata names that were used throughout those files. We then made two sets (one generic, one "type-aware") of `Meta` and `Child` objects for those nodes and metadata. The generic ones were in a namespace called `basic`, the type-aware ones in a namespace called `misc`. Given our later decision to separate GNDStk's Core Interface from individual Standard Interfaces that we'll have for each GNDS version, we no longer see the need to clutter the Core Interface with the `basic` and `misc` namespaces and all of their many `Meta` and `Child` objects. However, our test suite used some of the material in those namespaces, so we couldn't just remove it. In an earlier PR, we removed *unused* `Meta` and `Child` objects. A modest number of them, however, remained. In this PR, we've completely reworked the relevant material so that it still exists in its present form namespace-wise, but is no longer in the Core Interface. Instead, we have a new file, `src/GNDStk/test/keys.hpp`, that's included in test codes that need any of those old `Meta` and `Child` objects. But that file is *not* part of the Core Interface. This substantially declutters the Core Interface. The new `src/GNDStk/test/keys.hpp` file could arguably be split up. Right now, for example, it has several namespaces within it, which isn't consistent with our typical style, elsewhere in GNDStk, regarding source-file content. We may, therefore, split up `keys.hpp` sometime. For now, however, the file is included only in test-suite files, so we're not going to worry about it. The main point right now was to move, to a "test" location, material that we don't see as belonging in the Core Interface any longer. As a consequence of the changes we've made, we no longer have `namespace core`. Previously, `namespace core` had simply brought in (via using) the `njoy::GNDStk` namespace itself, and `njoy::GNDStk::basic` as well. The latter (`basic`) no longer exists in the main GNDS namespace, as described earlier. So, there's no point in having an `njoy::GNDStk::core` that just brings in `njoy::GNDStk`. The changes allowed for `GNDStk.hpp` itself to be somewhat decluttered. We also Removed the *empty* top-level GNDStk test. If we're going to have something there at all, we can put it in at another time. For now, having an empty test code just slowed the test-suite compilation. By removing it, we'll get a small but nonzero speedup in compilation. Content that was previously in `basic.hpp`, `basic/src/`, `misc.hpp`, `misc/src`, `common.hpp`, and `common/src` was moved to the brand-new file `test/keys.hpp`, the new file mentioned above. Autogenerated codes were updated to reflect all of the above.
Based on a user's experience with GNDStk, I reworked some of the details of how the `BlockData` class handles, in particular, the values of `length`, `start`, and `valueType`. Those values - `length`, `start`, and `valueType` - can appear in GNDS as metadata for elements that contain block data. Example: the `<values>` element. Our `BlockData` class is built so that it's able to handle all of those in a reasonable way. `BlockData` is a base of `Component`, which is always a base of some "high-level" class (as are produced by our code generator). A question arises about the need to "sync" `length`, `start`, and `valueType` in `BlockData` with whichever ones someone might choose to support (remember, the values aren't necessary) in a high-level class. Without going into details, we believe (sort of an editorial remark here) that having `length`, `start`, and `valueType` in GNDS introduces some goofiness, in terms of redundancies and such. If something is goofy, then its handling in any code is likely to be goofy as well. The changes we've made change from one plausible way of handling things, to another plausible way. The new scheme can't be characterized as perfect, and, given the nature of the values in question, no scheme is likely to be perfect. We believe, however, that our new scheme is less imperfect than what we had before, and it may (to be determined) fix the problem the user had. As a result of the changes, we were able to remove some SFINAE, and to clean up some code here and there. Relevant test codes were of course updated to reflect our changes.
Cleaned up some slight inconsistencies here and there. I'll write a detailed description in a pull request.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, GNDStk's Core Interface has a functional but basic HDF5 capability that can write an HDF5 file, but with all data (from either metadata, or from "data nodes" like
<values>...</values>) being written as strings.But we really want (perhaps as the default, or perhaps optional, to be determined) GNDStk to infer what type of data it's working with, whenever it's fixin' to create an HDF5 data set.
Examples:
A metadatum
foo="10"is probably meant to be one int.A metadatum
bar="10 20"is probably meant to be an array of int. (Aside: to help ensure accurate type guessing, such things should probably be written as shown - not written with a comma, as in"10,20", because the comma might throw a monkey wrench into our ability to guess type reliably.)A node
<values>1.23 4.56 7.89</values>is probably meant to be an array of doubles.A node
<values>H He Li</values>is probably meant to be an array of strings.The new HDF5
detail.hppfile has a simple (the simplest I could come up with) but effective "type guesser" that tries to make a basic determination as to what we're dealing with.In its present form, the type guesser can distinguish inputs with respect to these types:
Earlier types, in the above list, are preferred. For example,
"10 20 30"will be characterized as an array of ints - because it can be. On the other hand, on a machine with 32-bit ints,"10 20 4294967295"will be characterized as an array of unsigneds. (Not an array of ints, because the last number, 2 to the 32 power minus 1, requires an unsigned.)"10 20 4294967296"(without that minus 1!) will probably be an array of longs - not ints (it can't be), not unsigneds (it can't be), and not unsigned longs (it doesn't need to be).We don't bother with other types -
char, for example, orshort int- because we're not sure that our users will care about those in relation to HDF5 files. Also, HDF5 might not support all types we might distinguish in C++ code. The above list seems to be adequate. We can modify it later if we need to.Importantly, precisely how we use our new type guesser will probably depend on the context, in the Core Interface's representation of a GNDS hierarchy, of the string whose type we're guessing. Consider that the existing GNDS files we've seen typically have something like the following near the beginning:
Now consider that some GNDS files have something like this:
The behavior of our XML-file I/O capabilities is such that the first will end up as a string in a
cdatanode (technically named#cdatato call out its status as not-a-normal-node, but that's not important here). The second will end up as a string in apcdata(well,#pcdata) node.In XML, the distinction between CDATA and PCDATA is largely superficial, relating to how characters are parsed. However...
For the purposes of determining a data type in HDF5, our human brains tell us this:
For the first construct, we'd certainly want a single string.
For the second construct, we'll want an array of strings.
To an algorithm, unfortunately, both look like the same thing: strings separated by whitespace! So, our type guesser would say "array of strings" for both. But it would actually be ridiculous to split the first construct - the free-form descriptive string of the file and its contents - into an array of words and punctuation. So, we anticipate that the type guessing code will be used in a slightly different manner when we're looking at
pcdatathan it will be when we're looking atcdata. Which is fine; we just want to mention it.This pull request doesn't deal with issues like those quite yet. For now, we're just introducing the type guessing code, and of course testing it.
As often happens, working on this new material prompted me to notice some slight inconsistencies here and there, so I decided to fix those up. For example, the code base mostly says "unsigned" (shorter, which we prefer) as opposed to "unsigned int" (same meaning, but more verbose); but the latter did appear occasionally. Where doing so wouldn't change meaning, we changed the latter to the former. (Where we didn't change it, we were dealing with strings that are meant to be interpreted.)