Save Sample MeshObject to Nexus

[andy-bridger]

Description of work

Currently the only sample/environment shape information that is saved to a nexus file is the sample shape if it is a CSG shape. I have extended it to now also handle mesh shapes.

This is discussed in #28581 but this does not close that issue as I haven't added handling for the environment.

I would have liked to use some more of the existing tools, specifically the NX_OFF and the NexusGeometryParser discussed in the issue, but they cannot be brought into API::Sample (at least, I couldn't do it).

The solution I gone with seems deceptively simple, just saving the vertices and face arrays directly. Upon loading I just reconstruct the MeshObject by reading these arrays. I do have some uncertainty about the scope and ownership of the newly created object, but my hope is that the std::make_shared<Mantid::Geometry::MeshObject> takes care of that.

To test:

run the following script

# import mantid algorithms, numpy and matplotlib
from mantid.simpleapi import *
import matplotlib.pyplot as plt
import numpy as np

# update the paths for the mantid build directory and some temporary directory to save a file to
build_dir = r"C:\Users\kcd17618\Documents\dev\mantid\build"
tmp_dir = r"C:\Users\kcd17618\Documents\TestingTMP"

# test by loading a file without a shape, adding mesh from stl, saving, loading and checking for shape mesh 
ws = Load(fr"{build_dir}\ExternalData\Testing\Data\SystemTest\Texture\ValidationFiles\Focus\ENGINX_364901_361838_Texture30_dSpacing.nxs")

ws = LoadSampleShape(ws, Filename = fr"{build_dir}\ExternalData\Testing\Data\UnitTest\cube.stl")

SaveNexus(ws, Filename = fr"{tmp_dir}\test_ws.nxs")

reload_ws = Load(Filename = fr"{tmp_dir}\test_ws.nxs")

print("ws mesh: ", ws.sample().getShape().getMesh(), " new_ws mesh: ",reload_ws.sample().getShape().getMesh())

If you have any other ideas of where this might break things please test/ let me know

---

Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

• Is the code of an acceptable quality? (Code standards/GUI standards)
• Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
• Are appropriately scoped unit and/or system tests provided?
• Do the release notes conform to the guidelines and describe the changes appropriately?
• Has the relevant (user and developer) documentation been added/updated?
• If the PR author isn’t in the mantid-developers or mantid-contributors teams, add a review comment rerun ci to authorize/rerun the CI

Gatekeeper

As per the gatekeeping guidelines:

• Has a thorough first line review been conducted, including functional testing?
• At a high-level, is the code quality sufficient?
• Are the base, milestone and labels correct?

[MialLewis]

The Mantid::Nexus::Geometry library is not linked to Mantid::API.

You could do so here, if that would provide improvement?

[andy-bridger]

I'm not sure it would provide an improvement. The reasons I can see it being an improvement are:

1. if there is some inherent reason that having the specific NX_OFF data format is significantly more efficient than just saving the vertices and face arrays
2. if the MeshObject being created in loadNexus has some ownership issues

If I understand correctly it would probably be preferable to not introduce this link if possible, but I don't understand the above considerations well enough to say that it isn't needed (nor do I know what other considerations I have overlooked)

[MialLewis]

To help the reviewer, linked issue says The Nexus file format now supports mesh geometry using a format similar to OFF. Is this formalised in the NeXuS standard? Does this PR conform to the precedent set out in this implied support?

[andy-bridger]

To help the reviewer, linked issue says The Nexus file format now supports mesh geometry using a format similar to OFF. Is this formalised in the NeXuS standard? Does this PR conform to the precedent set out in this implied support?

I don't think I'm answering this (I'm not sure what NeXuS standard is, nor what precedent is being set out) but here is a brief overview of what I think the linked issue is suggesting the solution be

NX_OFF is a group defined in

mantid/Framework/NexusGeometry/inc/MantidNexusGeometry/NexusGeometryDefinitions.h

Line 27 in cd229b5

const H5std_string NX_OFF = "NXoff_geometry";

which is used by

mantid/Framework/NexusGeometry/src/NexusGeometryParser.cpp

Lines 662 to 682 in cd229b5

	std::shared_ptr<const Geometry::IObject> parseNexusShape(const Group &detectorGroup, bool &searchTubes) {
	// Note in the following we are NOT looking for named groups, only groups
	// that have NX_class attributes of either NX_CYLINDER or NX_OFF. That way
	// we handle groups called any of the allowed - shape, pixel_shape,
	// detector_shape
	auto cylindrical = utilities::findGroup(detectorGroup, NX_CYLINDER);
	auto off = utilities::findGroup(detectorGroup, NX_OFF);
	searchTubes = false;
	if (off && cylindrical) {
	throw std::runtime_error("Can either provide cylindrical OR OFF "
	"geometries as subgroups, not both");
	}
	if (cylindrical) {
	searchTubes = true;
	return parseNexusCylinder(*cylindrical);
	} else if (off)
	return parseNexusMesh(*off);
	else {
	return std::shared_ptr<const Geometry::IObject>(nullptr);
	}
	}

where parseNexusMesh unpacks the data stored in the NX_OFF group and hands it off to NexusShapeFactory::createFromOFFMesh (NX_OFF data is stored as 3 arrays: faces, vertices and winding order - vertices and winding order are basically what I am saving currently, with the extra faces array due to OFF supporting face geometries other than traingles, this means you need the faces array which provides the indices in winding order where each face beings {rather than just assuming it is a new face every three indices})

NexusShapeFactory::createFromOFFMesh then just does some leg work triangulating any faces that aren't already triangles but ultimately creates the MeshObject in the same way as done in this PR

mantid/Framework/NexusGeometry/src/NexusShapeFactory.cpp

Lines 182 to 198 in cd229b5

	std::unique_ptr<const Geometry::IObject> createFromOFFMesh(const std::vector<uint32_t> &faceIndices,
	const std::vector<uint32_t> &windingOrder,
	const std::vector<Eigen::Vector3d> &nexusVertices) {
	std::vector<uint32_t> triangularFaces = createTriangularFaces(faceIndices, windingOrder);
	return NexusShapeFactory::createMesh(std::move(triangularFaces), toVectorV3D(nexusVertices));
	}
	std::unique_ptr<const Geometry::IObject> createMesh(std::vector<uint32_t> &&triangularFaces,
	std::vector<Mantid::Kernel::V3D> &&vertices) {
	if (Geometry::MeshObject2D::pointsCoplanar(vertices))
	return std::make_unique<Geometry::MeshObject2D>(std::move(triangularFaces), std::move(vertices),
	Kernel::Material{});
	else
	return std::make_unique<Geometry::MeshObject>(std::move(triangularFaces), std::move(vertices), Kernel::Material{});
	}

So basically using NX_OFF requires adding support for converting the existing MeshObject into the 3 OFF arrays (generating the additional face index array which is arbitrary for our triangulated mesh) for them to be saved, just so they can be converted back into the vertex and face arrays that were available straight out of the MeshObject

So I would say the approach in this PR is more efficient, I think it is probably not formalised as a nexus standard though.

I suppose if we did go down the route of linking in Mantid::Nexus::Geometry we could make a NX_MESH with just the vertices and faces, extend parseNexusMesh to unpack these and hand them straight to createMesh, but hopefully you can appreciate that I didn't want to broadly assume I should mess with too much of the nexus infrastructure?

[rboston628]

In the MeshObject class, add a method saveNexus() that will handle saving these.

[rboston628]

In the MeshObject class, add a method loadNexus() that will handle these steps

[andy-bridger]

Further update, after discussions, I proposed these two solution options:

• link NexusGeometry into API (it seems a logical prerequisite at the point you are loading the sample and so want information about geometry from nexus - and Geometry is also already linked here) and then to add methods to MeshObject (to output the OFF data) and NexusGeometrySave (to save this to a  NX_OFF group) and have the NexusGeometrySave and NexusGeometryParser called from Sample to save/load
• Or ignore all that and just do what I've already done but move the functionality into MeshObject as described: Save Sample MeshObject to Nexus #40761 (comment)

Having looked into the first option I think it is even more convoluted as I think NexusGeometry pulls in API:

https://github.com/mantidproject/mantid/blob/cd229b57d8a0aa5c66df1dac93e8144db44f372a/Framework/NexusGeometry/CMakeLists.txt#L67C3-L67C38

So can't directly link without a circular dependency. For this reason I think the second option - packaging the existing changes into functions on mesh object, is the way to go?