diff --git a/reader/HDF5ImageLocator.cpp b/reader/HDF5ImageLocator.cpp index 1f9128699..4280276f0 100644 --- a/reader/HDF5ImageLocator.cpp +++ b/reader/HDF5ImageLocator.cpp @@ -74,7 +74,7 @@ HDF5ImageLocator::Location HDF5ImageLocator::Resolve(int64_t global_image) const if (!mapping.ContainsVirtualImage(image)) continue; return {OpenCached(mapping.filename), static_cast(mapping.SourceImage(image)), - mapping.filename}; + mapping.filename, mapping.dataset}; } throw JFJochException(JFJochExceptionCategory::HDF5, "Image not covered by /entry/data/data VDS mappings"); diff --git a/reader/HDF5ImageLocator.h b/reader/HDF5ImageLocator.h index 7b13cc06b..3d3f47f16 100644 --- a/reader/HDF5ImageLocator.h +++ b/reader/HDF5ImageLocator.h @@ -28,6 +28,9 @@ public: // Path the file was opened from. Needed to open it a second time as a plain file, for the // positional reads HDF5ImageSource does outside the mutex. std::string path; + // Where the images sit INSIDE that file. /entry/data/data everywhere DECTRIS writes, but a + // VDS names its source dataset and is free to name another one, so take it at its word. + std::string dataset = "/entry/data/data"; }; // Layout description, filled by the reader once the master file has been parsed. All paths diff --git a/reader/HDF5ImageSource.cpp b/reader/HDF5ImageSource.cpp index afca58fb3..9fc2d1a64 100644 --- a/reader/HDF5ImageSource.cpp +++ b/reader/HDF5ImageSource.cpp @@ -71,7 +71,7 @@ HDF5ImageLocator::Location HDF5ImageSource::Resolve(int64_t global) const { StoredPixelFormat HDF5ImageSource::GetStoredPixelFormat() const { auto loc = locator_.Resolve(0); - HDF5DataSet dataset(*loc.file, "/entry/data/data"); + HDF5DataSet dataset(*loc.file, loc.dataset); HDF5DataType datatype(dataset); return {static_cast(datatype.GetElemSize()) * 8, datatype.IsSigned()}; } @@ -85,12 +85,13 @@ std::vector HDF5ImageSource::GetSourceMapping(uint64_t fi const HDF5ImageSource::OpenDataset & HDF5ImageSource::GetDataset(const HDF5ImageLocator::Location &loc) const { - if (auto it = dataset_cache_.find(loc.file.get()); it != dataset_cache_.end()) + auto key = std::make_pair(loc.file.get(), loc.dataset); + if (auto it = dataset_cache_.find(key); it != dataset_cache_.end()) return it->second; OpenDataset entry; entry.file = loc.file; - entry.dataset = std::make_unique(*loc.file, "/entry/data/data"); + entry.dataset = std::make_unique(*loc.file, loc.dataset); HDF5DataSpace dataspace(*entry.dataset); HDF5DataType datatype(*entry.dataset); @@ -98,7 +99,7 @@ HDF5ImageSource::GetDataset(const HDF5ImageLocator::Location &loc) const { if (dataspace.GetNumOfDimensions() != 3) throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, - "/entry/data/data dataset must be 3D"); + loc.dataset + " dataset must be 3D"); if (datatype.IsFloat()) throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, @@ -115,20 +116,37 @@ HDF5ImageSource::GetDataset(const HDF5ImageLocator::Location &loc) const { if (entry.direct_chunk) entry.algorithm = dcpl.GetCompression(); - if (entry.direct_chunk && !loc.path.empty()) { - entry.raw = std::make_shared(loc.path); - if (!entry.raw->IsOpen()) - entry.raw.reset(); - hid_t fcpl = H5Fget_create_plist(loc.file->GetID()); - if (fcpl >= 0) { - hsize_t user_block = 0; - if (H5Pget_userblock(fcpl, &user_block) >= 0) - entry.user_block = user_block; - H5Pclose(fcpl); + if (entry.direct_chunk) { + // A positional read needs the file the PIXELS are in, and that is not always the file that + // was opened: a dataset reached through an external link lives elsewhere, and a chunk + // address is an offset into ITS file - as is the user block the address counts from. Ask + // HDF5 which file the dataset ended up in rather than assuming it is the one we asked. + std::string path = loc.path; + const hid_t owner = H5Iget_file_id(entry.dataset->GetID()); + if (owner >= 0) { + const ssize_t name_size = H5Fget_name(owner, nullptr, 0); + if (name_size > 0) { + std::string name(static_cast(name_size), '\0'); + if (H5Fget_name(owner, name.data(), name.size() + 1) >= 0) + path = name; + } + const hid_t fcpl = H5Fget_create_plist(owner); + if (fcpl >= 0) { + hsize_t user_block = 0; + if (H5Pget_userblock(fcpl, &user_block) >= 0) + entry.user_block = user_block; + H5Pclose(fcpl); + } + H5Fclose(owner); + } + if (!path.empty()) { + entry.raw = std::make_shared(path); + if (!entry.raw->IsOpen()) + entry.raw.reset(); } } - return dataset_cache_.emplace(loc.file.get(), std::move(entry)).first->second; + return dataset_cache_.emplace(std::move(key), std::move(entry)).first->second; } std::optional diff --git a/reader/HDF5ImageSource.h b/reader/HDF5ImageSource.h index a84334e2c..c4cda0ced 100644 --- a/reader/HDF5ImageSource.h +++ b/reader/HDF5ImageSource.h @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include #include "HDF5ImageLocator.h" @@ -126,6 +128,8 @@ private: CompressionAlgorithm algorithm = CompressionAlgorithm::NO_COMPRESSION; bool direct_chunk = false; }; - mutable std::map dataset_cache_; + // Keyed by file AND dataset path: a master whose VDS sources are datasets in itself serves + // several of them out of one file, and keying by file alone would hand back the wrong one. + mutable std::map, OpenDataset> dataset_cache_; const OpenDataset &GetDataset(const HDF5ImageLocator::Location &loc) const; }; diff --git a/reader/HDF5MetadataSource.cpp b/reader/HDF5MetadataSource.cpp index e2897b3e1..fe4889994 100644 --- a/reader/HDF5MetadataSource.cpp +++ b/reader/HDF5MetadataSource.cpp @@ -844,13 +844,19 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen detector.ReadOutTime(std::chrono::nanoseconds(0)); dataset->experiment.Detector(detector); + // frame_time is the period between frames, count_time the exposure within one. NXmx requires + // neither, and a master written outside the DECTRIS toolchain often carries only count_time; + // falling back to it says "no dead time", which is the honest reading of a file that does not + // state one. What is read here is metadata - the one place frame time is divided by is the + // JUNGFRAU summation, which a dataset read from a DECTRIS-style file never reaches. + const float count_time_s = master_file->GetFloat("/entry/instrument/detector/count_time"); dataset->experiment.FrameTime( std::chrono::duration_cast( std::chrono::duration( - master_file->GetFloat("/entry/instrument/detector/frame_time"))), + master_file->GetOptFloat("/entry/instrument/detector/frame_time") + .value_or(count_time_s))), std::chrono::duration_cast( - std::chrono::duration( - master_file->GetFloat("/entry/instrument/detector/count_time"))) + std::chrono::duration(count_time_s)) ); if (master_file->Exists("/entry/instrument/detector/calibration")) { @@ -878,9 +884,14 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen ReadROIMetadata(*master_file, *dataset); // Resolve VDS mapping filenames to absolute paths so the image source's locator only ever - // deals with real paths, then report the layout to the caller. + // deals with real paths, then report the layout to the caller. "." is HDF5's spelling for + // "the file this dataset is in", not a relative path - a master is allowed to compose its + // VDS over datasets in ITSELF, which are then external links to the data files. Resolved as + // a path it became /. and no image could be opened at all. for (auto &m : vds_data_mappings) - m.filename = ResolveRelativeToMaster(master_file_directory, m.filename); + m.filename = (m.filename == ".") + ? master_filename + : ResolveRelativeToMaster(master_file_directory, m.filename); dataset->experiment.ImagesPerTrigger(number_of_images); cached_geom = dataset->experiment.GetDiffractionGeometry(); diff --git a/tests/HDF5WritingTest.cpp b/tests/HDF5WritingTest.cpp index 48616c49a..535e13e55 100644 --- a/tests/HDF5WritingTest.cpp +++ b/tests/HDF5WritingTest.cpp @@ -98,6 +98,25 @@ TEST_CASE("HDF5DataSet_scalar", "[HDF5][Unit]") { REQUIRE (H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); } +// NXmx puts no rank on distance, saturation_value or an axis setting, and a writer outside the +// DECTRIS toolchain may store any of them as a length-1 array. That is the same one number. +TEST_CASE("HDF5DataSet_scalar_stored_rank1", "[HDF5][Unit]") { + { + HDF5File file("scratch1b.h5"); + file.SaveVector("one", std::vector{85.5}); + file.SaveVector("two", std::vector{85.5, 90.0}); + } + { + HDF5ReadOnlyFile file("scratch1b.h5"); + HDF5DataSet one(file, "one"); + CHECK(HDF5DataSpace(one).GetNumOfDimensions() == 1); + CHECK(one.ReadScalar() == 85.5); + REQUIRE_THROWS(HDF5DataSet(file, "two").ReadScalar()); + } + remove("scratch1b.h5"); + REQUIRE (H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); +} + TEST_CASE("HDF5DataSet_string", "[HDF5][Unit]") { std::string tmp_string = "HDF5Content"; diff --git a/writer/HDF5Objects.h b/writer/HDF5Objects.h index 741f3172d..d974a0ba5 100644 --- a/writer/HDF5Objects.h +++ b/writer/HDF5Objects.h @@ -281,8 +281,16 @@ public: template T ReadScalar() const { HDF5DataSpace mem_space({1}); HDF5DataSpace file_space(*this); - if (file_space.GetNumOfDimensions() != 0) - throw JFJochException(JFJochExceptionCategory::HDF5, "Dataset tries to read scalar from vector dataset");; + // One number does not have to be stored rank-0. NXmx puts no rank on distance, + // saturation_value or an axis setting, so a writer is free to store any of them as a + // length-1 array, and some do - which this rejected at the first field the reader touched. + // Anything holding exactly one element is that one number; only a real vector is an error. + hsize_t npoints = 1; + for (const auto dim : file_space.GetDimensions()) + npoints *= dim; + if (npoints != 1) + throw JFJochException(JFJochExceptionCategory::HDF5, + dataset_name + ": tries to read a scalar from a vector dataset"); T output{}; if (H5Dread(id, HDF5DataType(output).GetID(), mem_space.GetID(), H5S_ALL, H5P_DEFAULT, &output) < 0)