-
Notifications
You must be signed in to change notification settings - Fork 1
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/optional json #95
Open
staleyLANL
wants to merge
350
commits into
master
Choose a base branch
from
feature/optional-json
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
Additional tests for those, too.
There are still some instances of the strings in question, typically appearing amongst other text inside of string literals. At some point I'll look into fixing up those cases too, but that's not a priority right now.
Updated the HDF5 test suite to ensure that we test the modifications.
We'll update the code generator later, and ensure that it does the right thing. More feature-packed classes Field and FieldPart.
I'm not sure why I made them be converting constructors to begin with. Whatever the reason, it seems (according to the test suite!) that they don't need to be converting constructors. And, I don't think that they should be. Indeed, some new code modifications uncovered this issue, as converting constructors for Node triggered certain ambiguities.
Code generator itself still needs updating.
…in name isn't known. This was important for the new generated-class Field system to work well, without carrying along a std::string (expensive!) in each class field. On the other hand, such a thing wouldn't necessarily be a memory hog in a well-designed library that mostly contained vectors and such - not a huge number of individual scalar fields.
Improved some SFINAE. Added more comments, and did a bit of rearrangement.
It now generates classes that use the new system.
… feature/component-field
A variable CHAR_WIDTH appears in the HighFive HDF5 library, but CHAR_WIDTH is #defined in <limits>. The obvious consequental problem is fixed by our change.
Updated test code. Still a work-in-progress.
… feature/component-field
Added new material to a couple of test codes.
… offers. There's lots more to be done in the above respect; this is just a start. Also, provided some preliminary support for another approach to customization. The original (and still in place) "customization" system involves inserting code from custom.hpp files directly into generated classes. Inserting additional bits and pieces of code into already-existing classes is actually a somewhat goofy concept. (Imagine someone customizing, say, std::complex by adding a new piece of information. They really should derive from it, or contain it in another class. Not try to mess with the existing class.) I'm fleshing out *one* new way, at least (with possibly others to come) for doing customizations. There's more to be done, but I'll work with a particular user right now, to see how this works. JSON specs now allow metadata to have converters. It turns out that this will help with the new customization business. Briefly: we want an ability to insert our own fields into classes (say, to contain data that we'll compute as functions of other fields), but without those new fields playing a role in I/O from/to files. A new (and simple) "Noop" (no-op[eration]) class fills the necessary role of representing the "no conversion is taking place" for I/O on the new fields. Some new if-constexprs, elsewhere in the code, allow convert() functions to return a bool (but they can still return void, as before). If a convert() returns bool, and is used in the context of Node add() metadatum or child node, then a return value of false means, "don't actually add the new metadatum or children." This is all in support of our new customization approach, because we don't necessarily want auxiliary computed fields to be written to files. (But still want them in the multi-query, so they work with print() and such.) Tweaked some SFINAE. This detail:: stuff really ought to be made more internally consistent, even though it's in detail::. Note: the _v and _t business is modeled after what's in std::. (Examples: enable_if_t, is_convertible_v.) Adjusted some comments to reflect recent changes.
…terface. Some code cleanup and minor rearrangements. Lots more to come in the Python interface, as time allows. The next thing will probably be access to our extensive capabilities for controlling formatting, in particular as it relates to floating-point precision.
The code generator builds C++ code, a C-language interface, and Python bindings. Prior to this change, the code that generated the C interface handled only the old "BlockData" manner of handling data nodes. That was a construct that handled the "valueType"-based "run-time defined data type" business that exists in some GNDS nodes. But our format for format specs allows for a fixed data type as well, and it's handled in a different way. For a user who has been using GNDStk's code generator to generate an entirely different format, I needed to update the generation of the C interface in order to handle this properly.
These are largely in preparation for some upcoming, more-substantive changes; ones that should somewhat improve and clarify the interface.
Some work on Component::has() and Component::operator(). Small change: class to struct in a few places.
Additional work regarding Lookup objects, Field, Component, etc.
Cleaned up some SFINAE, and removed some that was no longer needed. A bit of formatting work in some CMakeLists.txt files, just to make them arguably more readable.
- Clarified "auto" in some places. - Ditto for constness.
Minor changes in some SFINAE to more-closely resemble certain std:: constructs. Sfome SFINAE sfimplifications. Removed unnecessary "inline"s. The functions in question were in fact function *templates*, so that "inline" was unnecessary in order to still achieve header-only-ness. Used "struct" instead of "class" in a few places. Used "size_t" instead of "int" in a few places. The difference wasn't really important in these places, but I decided that size_t was correct in principle. C Interface: fixed some previously unnoticed discrepancies that arose after an earlier change. Some function names were changed a while back: color to colors, note to notes, and warning to warnings. However, the *declarations* (for C) hadn't been updated accordingly. Function name change: detail::getter() to detail::compGetter(). (comp, for class Component.) Too many things were called "getter." This change should make code auditing/analysis a bit easier, going forward. Miscellaneous other changes. There are still some relevant files to be uploaded, but I need to review them more carefully first.
Split up some longer files.
The new overload and SFINAE were added after I switched from clang 9 to clang 17, and the latter reported overload ambiguities in some old code that hadn't had problems with earlier compilers. It's not clear to me that ambiguities really exist, but in any event, the new overload and the SFINAE disappears the new errors. Casted to (void) in certain places in some test files, after noticing that a new g++ emitted warnings about the results of some function calls being unused. (We did those calls to test that they're valid and to check out-parameters, not because we intended to use the return values of the functions.)
Under 1600 lines, in comparison with over 24000. Gives faster test-suite compilation times. Also, our JSON has more-flexible capabilities regarding the printing of floating-point numbers, in terms of format and number of significant digits. We're not taking advantage of those capabilities quite yet, but we will, if we choose to make JSON a first-class format - as important as XML.
Reduction in the amount of code. Should fix a compilation issue that arose in a user's generated C interface.
Use this: #define GNDSTK_DISABLE_HDF5 somewhere before including the main GNDStk header file, in order to switch off HDF5. Optionally, use the following with your compiler: -DGNDSTK_DISABLE_HDF5 Note that HDF5 is on, i.e. enabled, by default. When it's on, you need to link with HDF5 libraries on your computer. The above flag is thus needed in order to specifically *disable* HDF5. For our purposes, disabling HDF5 means you won't need to link with any HDF5 libraries. For technical reasons (in large part, to avoid extensive disruption of some internals in the GNDStk code base), it turns out that some "skeleton" HDF5 constructs still end up being in the code. Those constructs don't do anything useful, however, and don't require linking with the HDF5 libraries.
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.
JSON support is now optional.