diff --git a/common/DetectorSetup.cpp b/common/DetectorSetup.cpp index f6b2b3cc..73429dd8 100644 --- a/common/DetectorSetup.cpp +++ b/common/DetectorSetup.cpp @@ -74,8 +74,18 @@ DetectorSetup::DetectorSetup(std::shared_ptr in_geometry, break; case DetectorType::DECTRIS: high_voltage = 0; - bit_depth_readout = 16; - bit_depth_image = 16; + // Neither depth is set here. Both mean "the detector told us", and until something has + // asked it - the SIMPLON client at configure time, or the start message off the stream - + // nobody has. JUNGFRAU above keeps its 16, because there a 16-bit readout is a property + // of the hardware rather than something that gets configured. + // + // bit_depth_image in particular is deliberately left unset. It means "the detector told us what depth + // its images are", and until something has asked the detector - or read it off the + // stream - nobody has. Defaulting it to 16 made it look answered, and since + // GetByteDepthImage() gives this value absolute priority, that silently shadowed both + // the image-format setting the API offers and the promotion to 32 bits that summation + // needs. Unset, the fallback still lands on 16 bits for a DECTRIS detector, via the + // readout depth just above. read_out_time = std::chrono::microseconds(0); if (!det_modules_hostname.empty() && ( det_modules_hostname.size() != 1)) throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, diff --git a/common/DiffractionExperiment.cpp b/common/DiffractionExperiment.cpp index 1015bd0b..0b424a4e 100644 --- a/common/DiffractionExperiment.cpp +++ b/common/DiffractionExperiment.cpp @@ -392,9 +392,12 @@ int64_t DiffractionExperiment::GetByteDepthImage() const { auto bit_depth_image = image_format_settings.GetBitDepthImage(); if (!bit_depth_image.has_value()) { - if (GetBitDepthReadout() == 32) + // An unknown readout depth is not an error here - it just leaves the two-byte default below, + // which is what a DECTRIS detector reads out until it says otherwise. + const auto readout = GetBitDepthReadoutIfKnown(); + if (readout == 32) return 4; - if (GetBitDepthReadout() == 8) + if (readout == 8) return 1; return (GetSummation() > 2) ? 4 : 2; } @@ -669,7 +672,7 @@ void DiffractionExperiment::FillMessage(StartMessage &message) const { message.sensor_material = detector.GetSensorMaterial(); message.sensor_thickness = detector.GetSensorThickness_um() * 1e-6f; message.bit_depth_image = GetByteDepthImage() * 8; - message.bit_depth_readout = GetBitDepthReadout(); + message.bit_depth_readout = GetBitDepthReadoutIfKnown(); message.indexing_algorithm = GetIndexingAlgorithm(); message.images_per_trigger = dataset.GetImageNumPerTrigger(); @@ -1216,13 +1219,15 @@ int64_t DiffractionExperiment::GetImageFillValue() const { } } -int64_t DiffractionExperiment::GetBitDepthReadout() const { +std::optional DiffractionExperiment::GetBitDepthReadoutIfKnown() const { if (GetDetectorType() == DetectorType::EIGER) return GetEigerBitDepth(); + return detector.GetBitDepthReadout(); +} - auto det_value = detector.GetBitDepthReadout(); - if (det_value) - return det_value.value(); +int64_t DiffractionExperiment::GetBitDepthReadout() const { + if (auto value = GetBitDepthReadoutIfKnown()) + return value.value(); throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, "Bit depth readout not configured"); } diff --git a/common/DiffractionExperiment.h b/common/DiffractionExperiment.h index a1f3dafd..d7abd51b 100644 --- a/common/DiffractionExperiment.h +++ b/common/DiffractionExperiment.h @@ -203,6 +203,9 @@ public: DetectorMode GetDetectorMode() const; int64_t GetBitDepthReadout() const; // 12 bit is OK :) + // The same, for the callers that have to cope with not knowing it yet: a DECTRIS detector only + // learns its readout depth when something asks the detector or reads the start message. + std::optional GetBitDepthReadoutIfKnown() const; int64_t GetSaturationLimit() const; int64_t GetOverflow() const; diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7d83164f..81fc3068 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## 1.0.0 ### Unreleased +* jfjoch_broker: a DECTRIS detector no longer claims a 16-bit image and readout depth before anything has asked the detector. Both are now unset until the SIMPLON configuration or the incoming stream supplies them. **This makes `image_format_settings.bit_depth_image` work on DECTRIS detectors** - it was documented as controlling the output depth, but the detector's assumed default took priority and it was silently discarded. Where nothing supplies a depth the result is unchanged (16-bit). * **rugnux: 16-bit images keep their width all the way through the GPU pipeline** instead of being widened to 32-bit as soon as they are decoded. Every per-pixel pass over a frame - the ring statistics, the strong-pixel search, spot extraction, azimuthal and ROI integration, Bragg integration - then moves half as many bytes. This is the mode a fast acquisition reads out in, and the gain grows with the detector: on the 2.5M-pixel dataset in the test set the image loop drops 2%, and it is worth proportionally more on a large detector, where per-pixel work is most of the loop. Merged results are unchanged. * **rugnux: the first pass of the rotation two-pass no longer merges, reports or writes anything.** It exists to measure the detector geometry, the goniometer rotation scale, the mosaicity and the space group, and the second pass makes the merged result again at the refined geometry - so the first pass now stops once it has those. **The `_01_*` files are no longer written.** A battery of 24 rotation crystals drops from 6m52s to 6m17s and no merged result changes. * rugnux: every run ends with a `Time by phase` table - wall time and mean cores busy for each phase of the whole run, both passes together - so a slow dataset can be diagnosed from its own log without a profiler. diff --git a/tests/DetectorSetupTest.cpp b/tests/DetectorSetupTest.cpp index 5e81b8c5..606d0bed 100644 --- a/tests/DetectorSetupTest.cpp +++ b/tests/DetectorSetupTest.cpp @@ -4,6 +4,7 @@ #include #include "../common/DetectorSetup.h" #include "../common/NetworkAddressConvert.h" +#include "../common/DiffractionExperiment.h" TEST_CASE("DetectorSetup_MismatchInSize") { REQUIRE_THROWS( @@ -194,4 +195,34 @@ TEST_CASE("DetectorSetup_MaxFrameTime") { DetectorSetup s2(DetectorGeometryModular(8), DetectorType::EIGER); REQUIRE(s2.GetMinFrameTime() == MIN_FRAME_TIME_EIGER); -} \ No newline at end of file +} +// A DECTRIS detector starts with no image depth of its own, so the depth comes from what the run is +// actually configured to do. It used to default to 16, and because GetByteDepthImage() gives the +// detector's value absolute priority that silently shadowed everything below it - the image-format +// setting the API offers among them. +TEST_CASE("DetectorSetup_ImageDepthDefaultsToUnset") { + auto setup = DetDECTRIS(123, 123, "zzz", "a"); + REQUIRE(!setup.GetBitDepthImage()); + REQUIRE(!setup.GetBitDepthReadout()); + + // Nothing set anywhere: the fallback still lands on the readout depth, as before. + DiffractionExperiment x(setup); + REQUIRE(x.GetByteDepthImage() == 2); + + // The API's image-format setting is now reached instead of being discarded. + x.ImportImageFormatSettings(ImageFormatSettings().BitDepthImage(32)); + REQUIRE(x.GetByteDepthImage() == 4); + + // An unknown readout depth is not an error while the image depth is being decided, and it is + // reported as "not known" rather than invented. + REQUIRE_THROWS(x.GetBitDepthReadout()); + REQUIRE(!x.GetBitDepthReadoutIfKnown()); + setup.BitDepthReadout(32); + REQUIRE(DiffractionExperiment(setup).GetByteDepthImage() == 4); + + // And a depth the detector itself reports still wins over both. + setup.BitDepthImage(8); + DiffractionExperiment y(setup); + y.ImportImageFormatSettings(ImageFormatSettings().BitDepthImage(32)); + REQUIRE(y.GetByteDepthImage() == 1); +} diff --git a/tests/SpotExtractorGPUParityTest.cpp b/tests/SpotExtractorGPUParityTest.cpp index b5a86d9b..bdefce78 100644 --- a/tests/SpotExtractorGPUParityTest.cpp +++ b/tests/SpotExtractorGPUParityTest.cpp @@ -190,7 +190,7 @@ public: } std::vector Gpu(const SpotFindingSettings &settings) { std::vector spots; - extractor.Extract(gpu_strong.get(), image.getGPUBuffer(), settings, spots); + extractor.Extract(gpu_strong.get(), ViewOf(image), settings, spots); return spots; } };