Repository navigation
Custom hierarchies – other API concept, for now without extended meshes and particles path - #1881
Open
franzpoeschel wants to merge 85 commits into
Open
franzpoeschel wants to merge 85 commits into
franzpoeschel wants to merge 85 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| setDirty(false); | ||
| } | ||
| // customHierarchies().printRecursively(); |
| std::to_string(count_of_erased_elements) + " instead."); | ||
| } | ||
|
|
||
| // std::cout << "REWIRED '" << myPath().openPMDPath() << "'." << std::endl; |
franzpoeschel
force-pushed
the
custom-hierarchies
branch
3 times, most recently
from
May 20, 2026 12:27
036bd34 to
96307f3
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
June 10, 2026 08:45
233aab3 to
6bd551d
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
July 8, 2026 08:17
6bd551d to
3922027
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
2 times, most recently
from
July 20, 2026 13:11
9584ceb to
855ff65
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
August 19, 2026 15:16
3658077 to
08a1fb6
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
August 24, 2026 09:58
cc51fa4 to
13cee7f
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
August 27, 2026 08:37
e626d46 to
719b998
Compare
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
August 27, 2026 10:07
719b998 to
05c3ed4
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
2 times, most recently
from
September 8, 2026 13:54
9ce4f1f to
adb255b
Compare
franzpoeschel
force-pushed
the
custom-hierarchies
branch
2 times, most recently
from
October 8, 2026 12:46
2863f4e to
3519516
Compare
| RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) | ||
| { | ||
| setData(std::make_shared<Data_t>()); | ||
| setDatasetDefined(*m_recordComponentData); |
| REQUIRE(featureVariants.at("json") == true); | ||
| } | ||
|
|
||
| TEST_CASE("dataset_initialization", "[core]") |
franzpoeschel
force-pushed
the
custom-hierarchies
branch
2 times, most recently
from
October 13, 2026 09:52
a1a5537 to
2f3f64b
Compare
| REQUIRE(o.name() == "MyCustomOutput"); | ||
| } | ||
|
|
||
| TEST_CASE("optional_attribute_test", "[core]") |
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
October 15, 2026 14:34
2f3f64b to
bd35adb
Compare
Most of the .cpp file currently commented out
for more information, see https://pre-commit.ci
`find ../../../build/samples/` emits paths relative to the working directory, so the leading-slash patterns `/*custom_hierarchy/*` and `/*many_iterations/*` never matched. The custom-hierarchy sample was therefore validated against the openPMD JSON schema and failed on its non-standard keys (e.g. 'rabimmel'). Anchor the patterns to any path component so both directories are actually excluded.
Export Attributable/Series/CustomHierarchy classes on Windows The custom-hierarchies CoreTests link fails on Windows (MSVC/clang-cl) with LNK2001 for Attributable::visitHierarchyImpl. Annotating only the member function with OPENPMDAPI_EXPORT emits an /EXPORT for the method but not the class vftable, so the Container<...>/ConvertibleContainer<...> vtables emitted in the test still reference an unexported virtual function. Annotate the classes themselves with OPENPMDAPI_EXPORT instead, which exports the vftable/typeinfo as well, and drop the now-redundant member-level annotations. Export Container template instantiations on Windows The custom-hierarchies refactor added new virtual functions consumed by CoreTests. On Windows (MSVC/clang-cl), the explicit Container<...> template instantiations were not exported from openPMD.dll, causing LNK2001 for Attributable::visitHierarchyImpl. dllexport the explicit instantiations on MSVC. Export ConvertibleContainer instantiations on Windows The CoreTests custom_hierarchy test uses the datasets()/refresh()/subgroups() API of ConvertibleContainer<CustomHierarchy>/<CustomDataset>. On MSVC these template instantiations are not exported from openPMD.dll, causing LNK2001 for Attributable::visitHierarchyImpl via the emitted CustomHierarchy vtable. Export the explicit ConvertibleContainer instantiations on MSVC. Windows: Fix custom-hierarchy linker errors The Windows build failed to link CoreTests with LNK2001 for Attributable::visitHierarchyImpl and Series::visitHierarchyImpl. Root cause: the MSVC ABI encodes the member access specifier in the mangled name. Both virtuals were declared under the OPENPMD_protected / OPENPMD_private macros, but the invasive test translation units (CoreTest.cpp, Files_Core/custom_hierarchy.cpp) redefine those macros to 'public:'. The library therefore exported ...@@Meaa... (protected) while the vtables emitted by the test referenced ...@@UEAA... (public), so the symbol never resolved. Linux/Apple use the Itanium ABI, which ignores access specifiers in mangling, so only Windows was affected. Declare both members under an explicit access specifier instead of the macros. The accessibility is unchanged (still protected/private, matching the macros' defaults). Additionally, guard OPENPMDAPI_EXPORT on openPMD_EXPORTS so that consumers (including the tests) no longer see __declspec(dllexport), which would make MSVC expect these non-inline members to be defined in the consuming translation unit. The two class-level annotations from ab1d7dc are reverted to member-level; combined with the access-specifier fix and the export guard this exports the needed symbols correctly.
This reverts commit 6c5515b.
Container::insert(InputIt, InputIt) and BaseRecord::insert(InputIt, InputIt) forwarded to insert() on the SynchronizedContainers wrapper, which has no such member, so the overloads failed to compile for downstream users. - Implement the batch synchronization once in Container::insert(InputIt, InputIt) and delegate the initializer_list overload to it. - Define BaseRecord::insert(InputIt, InputIt) in the header. As a member template defined in the .cpp it was never instantiated, so it could not be linked by any consumer. - Cover both overloads in the existing baserecord_test and container_access_test cases.
The scalar-key guard erased the front element first and then read the key from the now-invalidated iterator to erase the corresponding backend entry. Copy the key and erase the backend entry before erasing the front element, in insert(value), insert(hint, value) and emplace().
BaseRecord::flush() is the sole caller of the flush_impl() overrides and unsets dirty flags centrally after flush_impl() returns. The TODO comments in Mesh::flush_impl and Record::flush_impl questioned why they do not unset dirty flags; document that this is intentional. PatchRecord::flush_impl was the outlier and called determineUnsetDirty redundantly; drop that call. Also fix the stale comment in BaseRecord::flush() that claimed the opposite.
Attributable::visitHierarchy() recurses into GroupMetaData::m_children_managed_as_custom_hierarchy after calling visitHierarchyImpl(). CustomHierarchy::visitHierarchyImpl() in turn traversed its whole frontend container, which is a superset of the managed map (it also aliases objects of the regular openPMD hierarchy), so objects created by a custom hierarchy were visited at least twice and their container nodes could be skipped. Traverse only the frontend children that are not part of the managed map and emit the node itself, leaving managed children to the base class. Extend the existing custom_hierarchy test to assert that every object is visited exactly once.
operator++(int) and operator--(int) returned a reference to the iterator *after* the increment, so it! and it-- did not yield the previous position as required by the iterator contract. Return a copy of the old position by value. Add coverage to the existing container_access_test.
- getAttributeOptional was documented as throwing no_such_attribute_error,
but it returns an empty optional by design.
- The Container static_assert still referred to Writable although the checked
trait was renamed to check for Attributable.
- Replace a std::runtime_error("Unreachable!") in customHierarchyFlush with
error::Internal and a descriptive message.
ConvertibleContainer::asDataset() checked that a written group has no children before reinterpreting it as a (constant) dataset, but the branch for objects that have not been written yet called initDataset() unconditionally. Casting a non-empty, unwritten group silently replaced its group metadata (discarding the contained objects) and left the object in an inconsistent state. Guard that branch as well and cover it in the custom_hierarchy test.
Cover Attributable::getAttributeOptional and Series::meshesPathOptional / particlesPathOptional for both present and absent attributes, including that getAttribute still throws for missing keys.
Container::erase() and clear() removed entries from the frontend container and from the backend group's children map, but not from GroupMetaData::m_children_managed_as_custom_hierarchy. Objects created via CustomHierarchy were therefore re-created (and re-written) upon the next flush after being erased. Drop the corresponding entry from the managed storage as well and cover erase with a flush/reopen round trip.
Summarize the bugs found and fixed during review (HierarchyVisitor double traversal, iterator post-increment contract, asDataset on non-empty groups, erase leaving custom-hierarchy storage stale), the documentation fixes, and the remaining API gaps and design concerns.
This reverts commit d4a89bd.
Several members that were only meant for internal use were exposed on the public API surface: - Attributable::customHierarchyFlush takes an internal::FlushParams and an internal managed_as_custom_object flag. Move it from public to protected; existing callers are all Attributable members or derived types. - CustomHierarchy and CustomDataset were publicly default-constructible, producing detached objects without IOHandler, parent or backend object. Make CustomHierarchy() protected and CustomDataset() private, granting Container/ConvertibleContainer/CustomHierarchy/Attributable friendship as needed (mirroring RecordComponent).
franzpoeschel
force-pushed
the
custom-hierarchies
branch
from
October 21, 2026 10:50
bd35adb to
0ebe80a
Compare
…nal state (#14) * Add internal ChildrenCache for type-erased child maps * Wire ChildrenCache into group child storage * Add backend-to-frontend child materialization path * Remove per-handle fallback container storage * Remove obsolete container synchronization plumbing * Fix erase counting * Add temporary review notes for children-cache * Traverse authoritative children through projection views * Clarify ChildrenCacheMap mutation restrictions * cleanup * Refine child cache and projection view interfaces * Reject non-canonical integral child keys * Simplify key conversion and iterator state * Use virtual structures for entries instead of C style * remove plans --------- Co-authored-by: AI Agent <ai-agent@localhost>
| } | ||
| } // namespace | ||
|
|
||
| auto children_cache() -> void |
| drain_set::drain_set(); | ||
| } | ||
|
|
||
| TEST_CASE("children_cache_test", "[auxiliary]") |
| read_nonexistent_attribute::read_nonexistent_attribute(); | ||
| } | ||
|
|
||
| TEST_CASE("custom_hierarchy", "[core]") |
| custom_hierarchy::custom_hierarchy(); | ||
| } | ||
|
|
||
| TEST_CASE("custom_hierarchy_materialization_error", "[core]") |
| custom_hierarchy::materialization_error(); | ||
| } | ||
|
|
||
| TEST_CASE("custom_hierarchy_iteration_filtering", "[core]") |
This branch has not been deployed
This file contains hidden or 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
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.
tbd
TODO:
Diff: franzpoeschel/openPMD-api@fix-parallel-hdf5...custom-hierarchies