Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/io revamp #80

Open
wants to merge 72 commits into
base: develop
Choose a base branch
from
Open

Feature/io revamp #80

wants to merge 72 commits into from

Conversation

staleyLANL
Copy link
Contributor

This began as some work to ensure smooth operation and consistency between various I/O-related constructs (for xml, json, and hdf5), but seems to have branched into various other things as well. I'd better just put it all in, before wandering completely into the wilderness with more work.

staleyLANL and others added 30 commits November 1, 2021 22:19
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.
… respect to references.

Function has<Lookup<false>> creates has<Lookup<true>>; will be used for queries.

Tweaks to type names, parameter names, and some formatting.

Added some traits classes; will use them soon.
Necessary modifications to class Lookup.

Updated the code generator.

Tedious and sometimes difficult Component::getter() modifications to make it all work smoothly. Return types had to be more general, and some new SFINAE was in order.

Ditto, even more so, for the detail::getter() functions. Lots of new SFINAE, function templates, and other constructs. The goal is ultimately to support some nifty new capabilities in Component-derived classes, in particular those that are generated with the code generator. Some separate (from GNDStk proper) work I have seems to indicate that the new constructs work as intended, but lots of tests, *in* GNDStk, should be written. Also, I'll need to run the code generator to regenerate our GNDS Standard Interface prototype. This isn't included quite yet, in this commit, because I'll want more time to review and validate what it generates.
…at it deals with the presence of member function templates (not just member function non-templates) in the generated C++ classes.
Both Node-to-HDF5 and5 HDF-to-Node (write, and read) now have the ability to deal with original string-form content (as from an original XML file, in GNDStk's Node), and "type-ified" versions thereof. For example, `"1.2 3.4"` is a string, but for HDF5 we can write it as two doubles. Also, importantly, content related to "special" nodes, in particular what we'd see in XML as CDATA, comments, and PCDATA, are now handled in a full, and I believe proper, manner. For both input and output of HDF5.

At some point I'll have to document all the HDF5 capabilities. For now, for anyone who wants to play around with HDF5, I recommend trying different combinations of the HDF5::reduced and HDF5::typed boolean flags; and then using perhaps the `h5dump` tool, or any other good HDF5 tool, for seeing what GNDStk's HDF5 capabilities can produce.

Also: I filled out the main HDF5 test code with tons of new tests. This was definitely needed at this point.

Also...

Changed some HDF5-related function names that are in the detail:: namespace. The earlier names had tried to use capitalization (e.g. HDF5 instead of hdf5) in a manner that was consistent with the non-detail:: API names. For detail stuff, however, certain capitalization and long names felt heavy and painful to the eyes.

Added a couple of tests for constructs that, it turned out, had not been interpreted correctly by the "type guesser" algorithm that the generic Node-to-HDF5 conversion uses.

Did a few things that were unrelated to the main work (HDF5) here, but I was
thinking about them...

Tweaked a couple of autogen files, per some remarks about earlier pull requests.

Figured out how to provide certain *constexpr* `has()` functions for classes
derived from `Component`. We anticipate that these may eventually prove to be
quite useful. Importantly, these are fully in `class Component` itself. Nothing
had to be added to the generated classes. Things like this is why `Component`
is awesome.
FileType::null to FileType::guess
FileType::text to FileType::debug
This is a prerequisite for some upcoming work.
Could replace fileName with filePtr->getName().
Basically, the former was redundant.
Possible in part because of some other recent work.
For example, >> and << (relative to streams and to strings) for Component.
Slightly changed some names and parameters here and there, to make things more consistent.
That's really the right thing to do (until/unless they fix it).
XML, JSON, etc. are now more consistent - no spurious newline in XML.
Updated test codes accordingly.

Fixed some fixmes.

General detail work, with the aim of tightening up the code.

Tweaked some comments.

More Node/Tree consolidation.
We use Node in most places, Tree only when necessary.

Removed some "inline"s when the function was a template anyway. Inline doesn't really mean "inline it" these days (compilers are good at making that decision), and it isn't necessary for header-only if the function is a template.
Rearranged some code.
Tweaked some terminology.
In the generated classes, and the code that works with them, struct content is now struct Content. That is, we just capitalized Content.

This simple change is consistent with NJOY classes generally having capitalized names.

More importantly, perhaps, I'm trying to make the code generator allow for as much flexibility as reasonably possible, given that users might be designing their own data format. (Not just using the code generator to build GNDS version-specific data structures.)

If someone actually writes a code-generator input spec that has a node called "content", it'll work now. :-) Not that we really expect that someone would call anything "content", but it *would* be a completely reasonable node name. By using upper-case terms for what we generate automatically - aside from terminology in someone's spec - people can use lower-case words in their specs and be confident that no conflict will arise.

That's the main thinking behind this simple name change.
Made miscellaneous small improvements in comments, names, etc.

BlockData now has implicit conversion to vector, when warranted.
It can be used where .get() was previously required.

Fixed the performance issue when default-constructing certain generated classes under certain circumstances.

Fixed some outdated (so, wrong) comments and strings in a couple of the test codes.
That fixed a problem with certain initializations being very time-consuming.
Properly split out and clarified "write" versus "print".
Removed some old debug messages.
Changed some terminology.
Trimmed some unused stuff from the KeyTuple (formerly KeywordTup) class.
Fix (I hope) error in g++ compilation.
@staleyLANL staleyLANL requested a review from whaeck March 29, 2022 20:12
@staleyLANL staleyLANL self-assigned this Mar 29, 2022
@whaeck whaeck changed the base branch from master to develop June 30, 2022 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants