Bugfix for regressions when loading sidre files in VisIt by kennyweiss · Pull Request #1792 · llnl/axom

kennyweiss · 2026-02-03T04:46:06Z

Summary

This PR contains bugfixes for regressions we noticed when saving visualization files from Sidre following the mesh blueprint convention.

In Deep copy from Sidre to Conduit #1510, Sidre's View class changed its internal state representation for non-array types from "SCALAR" to "TUPLE" and removed an internal sidre::View::State::SCALAR type.
This caused a problem loading these files into VisIt
This PR adds a config variable AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING for backward compatibility until VisIt can load Sidre files with "TUPLE" types. This option is ~~OFF~~ ON by default. When enabled, it will use "SCALAR" instead of "TUPLE" for views containing a single item.
- Update: Based on PR review feedback, I have enabled this option by default which will hopefully avoid problems until there is greater update for the TUPLE construct.
The following VisIt PR will add support for "TUPLE": add support to blueprint plugin to read sidre tuple views visit-dav/visit#20811 (thanks @cyrush !)
This PR also updates the Sidre Fortran interface for sidre::Group::createViewScalar() and sidre::Group::createViewString() to allow users to specify an allocator ID for the view's memory. These overloads were added in the same PR (Deep copy from Sidre to Conduit #1510)

…eViewString()` with allocators

…hroud

sidre::View's State enum removed the "SCALAR" type in favor of "TUPLE" and VisIt does not yet recognize these files. This commit adds a shim to output SCALAR for TUPLE types.

This will output "SCALAR" instead of "TUPLE" for tuple views in support of downstream readers of sidre files, like VisIt. We expect to phase this option out over time as application adopt the tuple type.

The functionality was moved into the hpp file a long time ago when templates were added to the class, but this file was not deleted.

rhornung67 · 2026-02-03T16:44:04Z

src/axom/sidre/core/SidreTypes.hpp

+inline constexpr int getInvalidAllocatorID() noexcept { return axom::INVALID_ALLOCATOR_ID; }
+
+/*!
+ * \brief Return Axom's malloc allocator id sentinel.


IIRC, the way allocator's work in Sidre is that you can attach one to a Group or View in the hierarchy and then it will be used for data allocation in the entire subtree rooted at the group (unless another allocator is attached to Group in the subtree) or View, respectively. Is this sentinel the default allocator if none is associated with a Group or View as I've described?

The default is INVALID_ALLOCATOR_ID which eventually maps to Umpire's default (when using Umpire) or malloc (when not using Umpire.

See:

axom/src/axom/core/memory_management.hpp

Lines 114 to 127 in 676ca97

/*!

* \brief Returns the ID of the current default Umpire allocator

* or MALLOC_ALLOCATOR_ID if Umpire is not used.

*

* \return ID of the current Umpire default allocator or MALLOC_ALLOCATOR_ID.

*/

inline int getDefaultAllocatorID()

{

#ifdef AXOM_USE_UMPIRE

return umpire::ResourceManager::getInstance().getDefaultAllocator().getId();

#else

return MALLOC_ALLOCATOR_ID;

#endif

}

and

axom/src/axom/sidre/core/Group.cpp

Lines 2285 to 2311 in 676ca97

Group::Group(const std::string& name, DataStore* datastore, bool is_list)

: m_name(name)

, m_index(InvalidIndex)

, m_parent(nullptr)

, m_datastore(datastore)

, m_is_list(is_list)

, m_view_coll(nullptr)

, m_group_coll(nullptr)

#ifdef AXOM_USE_UMPIRE

, m_default_array_alloc_id(axom::getDefaultAllocatorID())

, m_default_tuple_alloc_id(axom::getDefaultAllocatorID())

#else

, m_default_array_alloc_id(axom::MALLOC_ALLOCATOR_ID)

, m_default_tuple_alloc_id(axom::MALLOC_ALLOCATOR_ID)

#endif

{

if(is_list)

{

m_view_coll = new ListCollection<View>();

m_group_coll = new ListCollection<Group>();

}

else

{

m_view_coll = new MapCollection<View>();

m_group_coll = new MapCollection<Group>();

}

}

rhornung67

One question about concept clarification.

cyrush

Added a few comments, looks good.

Even if the new I/O selection option goes away for writing, we should also keep the case that maps reading "SCALAR" to "TUPLE" to help read old files.

src/axom/sidre/core/View.cpp

cyrush · 2026-02-03T17:14:33Z

src/axom/sidre/core/View.cpp

+// data (setScalar()) was serialized as "SCALAR". Some downstream readers
+// (e.g. VisIt's Blueprint database plugin) still expect that string.
+#if defined(AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING)
+  if(m_state == TUPLE && isScalar()) return "SCALAR";


If we save a tuple that is actually an array - VisIt's released logic would still likely work if you also use "SCALAR".

TUPLE as array could happen for some Mesh metadata -- not sure the probability of it occurring.

@gunney1 -- what did we do for small tuple arrays before your changes in #1510 -- I think we treated those as BUFFER arrays, so we don't have those cases. (I'll check)

I think that is true, it wasn't possible before -- but it is possible now -- in the past they would be attached to a buffer, but now they could be tagged TUPLE.

If tagged with SCALAR for the output file -- it may work in VisIt b/c that case we simply set using the inline values.

However the visit logic does not apply a schema, like it would the buffer case.

I think that before #1510 anything with more than 1 element is an array (unless it's a string), so it's stored in a BUFFER (unless it's external).

cyrush · 2026-02-03T17:38:26Z

Also, should the default be ON for the write option?

kennyweiss · 2026-02-03T18:34:38Z

Also, should the default be ON for the write option?

I was thinking it would be opt-in. Do you think we should enable it by default now and then disable it later when people are on the new VisIt?

Update: I changed it to ON by default

Arlie-Capps · 2026-02-03T19:29:37Z

src/cmake/AxomOptions.cmake

+# Sidre will serialize these views with state="TUPLE".
+option(AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING
+       "Write sidre View scalars with state='SCALAR' (legacy compatibility); default writes state='TUPLE'."
+       OFF)


I prefer default ON because the need for OFF (that is, using "TUPLE" instead of "SCALAR") is a small new population. The old default should keep working for everyone else, correct? Keeping the old default but letting those who need it set the new behavior is the gentlest way forward. Plus it allows us to test in a somewhat more controlled manner.

Arlie-Capps

@kennyweiss , thank you for this expeditious and thorough PR. Please consider having AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING default to ON: more discussion at AxomOptions.cmake.

Based on reviewer feedback, this seems like the safest strategy until there is better adoption of the TUPLE construct

kennyweiss and others added 9 commits February 2, 2026 20:32

Adds shroud overloads to sidre::Group createViewScalar() and `creat…

3399945

…eViewString()` with allocators

Adds functions to expose Axom's invalid and malloc allocator IDs to s…

a6461e0

…hroud

Updates shroud interface

ec298e9

Adds Fortran unit test for sidre::Group's functions w/ allocators

8d6bb9d

Adds View::getIoStateStringName() for backwards compatibility with VisIt

6b94159

sidre::View's State enum removed the "SCALAR" type in favor of "TUPLE" and VisIt does not yet recognize these files. This commit adds a shim to output SCALAR for TUPLE types.

Adds CMake config options AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING

3c2c9ae

This will output "SCALAR" instead of "TUPLE" for tuple views in support of downstream readers of sidre files, like VisIt. We expect to phase this option out over time as application adopt the tuple type.

Removes outdated cpp file from slam

a9fb956

The functionality was moved into the hpp file a long time ago when templates were added to the class, but this file was not deleted.

Use tmp files for new sidre tests

fdd7d10

Updates RELEASE-NOTES

1694631

kennyweiss added this to the FY26 January Release milestone Feb 3, 2026

kennyweiss requested review from Arlie-Capps, BradWhitlock, bmhan12, cyrush, gunney1, jcs15c, nselliott, publixsubfan, rhornung67 and white238 February 3, 2026 04:46

kennyweiss self-assigned this Feb 3, 2026

kennyweiss added bug Something isn't working App Integration Issues related to integration with applications Sidre Issues related to Axom's 'sidre' component Build system Issues related to Axom's build system labels Feb 3, 2026

rhornung67 reviewed Feb 3, 2026

View reviewed changes

cyrush approved these changes Feb 3, 2026

View reviewed changes

rhornung67 approved these changes Feb 3, 2026

View reviewed changes

Arlie-Capps reviewed Feb 3, 2026

View reviewed changes

Arlie-Capps approved these changes Feb 3, 2026

View reviewed changes

Enables AXOM_SIDRE_IO_USE_SCALAR_STATE_STRING by default

a4fba50

Based on reviewer feedback, this seems like the safest strategy until there is better adoption of the TUPLE construct

kennyweiss merged commit d732a8e into develop Feb 4, 2026
15 checks passed

kennyweiss deleted the bugfix/kweiss/update-sidre-shroud branch February 4, 2026 04:39

	/*!
	* \brief Returns the ID of the current default Umpire allocator
	* or MALLOC_ALLOCATOR_ID if Umpire is not used.
	*
	* \return ID of the current Umpire default allocator or MALLOC_ALLOCATOR_ID.
	*/
	inline int getDefaultAllocatorID()
	{
	#ifdef AXOM_USE_UMPIRE
	return umpire::ResourceManager::getInstance().getDefaultAllocator().getId();
	#else
	return MALLOC_ALLOCATOR_ID;
	#endif
	}

	Group::Group(const std::string& name, DataStore* datastore, bool is_list)
	: m_name(name)
	, m_index(InvalidIndex)
	, m_parent(nullptr)
	, m_datastore(datastore)
	, m_is_list(is_list)
	, m_view_coll(nullptr)
	, m_group_coll(nullptr)
	#ifdef AXOM_USE_UMPIRE
	, m_default_array_alloc_id(axom::getDefaultAllocatorID())
	, m_default_tuple_alloc_id(axom::getDefaultAllocatorID())
	#else
	, m_default_array_alloc_id(axom::MALLOC_ALLOCATOR_ID)
	, m_default_tuple_alloc_id(axom::MALLOC_ALLOCATOR_ID)
	#endif
	{
	if(is_list)
	{
	m_view_coll = new ListCollection<View>();
	m_group_coll = new ListCollection<Group>();
	}
	else
	{
	m_view_coll = new MapCollection<View>();
	m_group_coll = new MapCollection<Group>();
	}
	}

Conversation

kennyweiss commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

rhornung67 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

kennyweiss Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

rhornung67 left a comment

Choose a reason for hiding this comment

Uh oh!

cyrush left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cyrush Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

kennyweiss Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

cyrush Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gunney1 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

cyrush commented Feb 3, 2026

Uh oh!

kennyweiss commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arlie-Capps Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

kennyweiss Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Arlie-Capps left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kennyweiss commented Feb 3, 2026 •

edited

Loading

cyrush left a comment •

edited

Loading

cyrush Feb 3, 2026 •

edited

Loading

kennyweiss commented Feb 3, 2026 •

edited

Loading