diff --git a/instrument/unit_testing/REFL_Parameters_Analysis_Invalid.xml b/instrument/unit_testing/REFL_Parameters_Analysis_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..3aee9d4f46a9e9dd83c87ab235a6d93c48295c40 --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_Analysis_Invalid.xml @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="AnalysisMode" type="string"> + <value val="badAnalysisOption"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/instrument/unit_testing/REFL_Parameters_Correction_Invalid.xml b/instrument/unit_testing/REFL_Parameters_Correction_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..070e9f30542ce57e342636402b9d06646288297c --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_Correction_Invalid.xml @@ -0,0 +1,16 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="PolarizationAnalysis" type="string"> + <value val="badPolarizationOption"/> + </parameter> + + <parameter name="FloodCorrection" type="string"> + <value val="badFloodOption"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/instrument/unit_testing/REFL_Parameters_Debug_Invalid.xml b/instrument/unit_testing/REFL_Parameters_Debug_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..afc0bc73464fb80a57531d84a5619a0a55867b05 --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_Debug_Invalid.xml @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="Debug"> + <value val="badDebugOption"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/instrument/unit_testing/REFL_Parameters_PerTheta_Invalid.xml b/instrument/unit_testing/REFL_Parameters_PerTheta_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..a4e0c9380c6b741132e6186c942b96d98d261263 --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_PerTheta_Invalid.xml @@ -0,0 +1,28 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="QMin"> + <value val="0.1"/> + </parameter> + + <parameter name="dQ/Q"> + <value val="badResolution"/> + </parameter> + + <parameter name="QMax"> + <value val="0.02"/> + </parameter> + + <parameter name="ScaleFactor"> + <value val="badScale"/> + </parameter> + + <parameter name="ProcessingInstructions" type="string"> + <value val="badInstructions"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/instrument/unit_testing/REFL_Parameters_Reduction_Invalid.xml b/instrument/unit_testing/REFL_Parameters_Reduction_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..0f087ece856c7ed9e7fa4f78d6167644782f8e5f --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_Reduction_Invalid.xml @@ -0,0 +1,20 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="SummationType" type="string"> + <value val="badSummationOption"/> + </parameter> + + <parameter name="ReductionType" type="string"> + <value val="badReductionOption"/> + </parameter> + + <parameter name="IncludePartialBins"> + <value val="badPartialBinsOption"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/instrument/unit_testing/REFL_Parameters_TransmissionRunRange_Invalid.xml b/instrument/unit_testing/REFL_Parameters_TransmissionRunRange_Invalid.xml new file mode 100644 index 0000000000000000000000000000000000000000..0cc70f11e40131b021cec6f48c21016f7ea631c5 --- /dev/null +++ b/instrument/unit_testing/REFL_Parameters_TransmissionRunRange_Invalid.xml @@ -0,0 +1,16 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<parameter-file instrument="REFL" valid-from="forever"> + +<component-link name="REFL" > + + <parameter name="TransRunStartOverlap"> + <value val="12.0"/> + </parameter> + + <parameter name="TransRunEndOverlap"> + <value val="10.0"/> + </parameter> + +</component-link> + +</parameter-file> diff --git a/qt/scientific_interfaces/CMakeLists.txt b/qt/scientific_interfaces/CMakeLists.txt index 9bfd89c50d9257fda8790884b89288227f7c7976..2e3bcaaa3ac011a06e2f0d4f0d980b6a6312a7bb 100644 --- a/qt/scientific_interfaces/CMakeLists.txt +++ b/qt/scientific_interfaces/CMakeLists.txt @@ -55,6 +55,7 @@ set(TEST_FILES test/ISISReflectometry/RunsTable/RunsTablePresenterRowDeletionTest.h test/ISISReflectometry/Event/EventPresenterTest.h test/ISISReflectometry/Experiment/ExperimentPresenterTest.h + test/ISISReflectometry/Experiment/ExperimentOptionDefaultsTest.h test/ISISReflectometry/Experiment/PerThetaDefaultsTableValidatorTest.h test/ISISReflectometry/Instrument/InstrumentPresenterTest.h test/ISISReflectometry/Runs/RunsPresenterTest.h diff --git a/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.cpp b/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.cpp index bf39e41f9a941b51e333a51ba64c80b64b6de8a1..f54a80c155f5ec0e55cb3767bfbcd96807c96db8 100644 --- a/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.cpp +++ b/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.cpp @@ -42,6 +42,13 @@ OptionDefaults::getStringOrDefault(std::string const &propertyName, defaultValue); } +std::string +OptionDefaults::getStringOrEmpty(std::string const &propertyName, + std::string const ¶meterName) const { + return getValueOrDefault<std::string>(propertyName, parameterName, + ""); +} + std::string OptionDefaults::getString(std::string const &propertyName, std::string const ¶meterName) const { return getValue<std::string>(propertyName, parameterName); diff --git a/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.h b/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.h index aae9b421807ac618abb4be615e5b019d5e19e466..5be6179d669f380e24e36d702c1fac625b7f22b4 100644 --- a/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.h +++ b/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.h @@ -41,6 +41,8 @@ public: std::string getStringOrDefault(std::string const &propertyName, std::string const ¶meterName, std::string const &defaultValue) const; + std::string getStringOrEmpty(std::string const &propertyName, + std::string const ¶meterName) const; std::string getString(std::string const &propertyName, std::string const ¶meterName) const; diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentOptionDefaults.cpp b/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentOptionDefaults.cpp index b98b1beb1161a8659b8be267613386d2acfceb05..a91c8d0bda131af0e98f3f856012401ec44c9d8a 100644 --- a/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentOptionDefaults.cpp +++ b/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentOptionDefaults.cpp @@ -8,9 +8,20 @@ #include "Common/OptionDefaults.h" #include "MantidAPI/AlgorithmManager.h" #include "Reduction/Experiment.h" +#include "PerThetaDefaultsTableValidator.h" namespace MantidQt { namespace CustomInterfaces { + +// unnamed namespace +namespace { +Mantid::Kernel::Logger g_log("Reflectometry GUI"); + +std::string stringValueOrEmpty(boost::optional<double> value) { + return value ? std::to_string(*value) : ""; +} +} + Experiment experimentDefaults(Mantid::Geometry::Instrument_const_sptr instrument) { auto defaults = OptionDefaults(instrument); @@ -42,34 +53,45 @@ experimentDefaults(Mantid::Geometry::Instrument_const_sptr instrument) { defaults.getDoubleOrZero("StartOverlap", "TransRunStartOverlap"), defaults.getDoubleOrZero("EndOverlap", "TransRunEndOverlap")); + if (!transmissionRunRange.isValid(false)) + throw std::invalid_argument("Transmission run overlap range is invalid"); + // We currently don't specify stitch parameters in the parameters file // although we auto stitchParameters = std::map<std::string, std::string>(); // For per-theta defaults, we can only specify defaults for the wildcard row - // i.e. - // where theta is empty. It probably doesn't make sense to specify - // tranmsission runs - // so leave that empty. - auto theta = boost::none; - auto transmissionRuns = TransmissionRunPair(); - auto qRange = RangeInQ( - defaults.getOptionalValue<double>("MomentumTransferMin", "QMin"), - defaults.getOptionalValue<double>("MomentumTransferStep", "dQ/Q"), - defaults.getOptionalValue<double>("MomentumTransferMax", "QMax")); - auto scaleFactor = + // i.e. where theta is empty. It probably doesn't make sense to specify + // tranmsission runs so leave that empty. + auto const theta = std::string(""); + auto const firstTransmissionRun = std::string(""); + auto const secondTransmissionRun = std::string(""); + auto const qMin = stringValueOrEmpty(defaults.getOptionalValue<double>( + "MomentumTransferMin", "QMin")); + auto const qMax = stringValueOrEmpty(defaults.getOptionalValue<double>( + "MomentumTransferMax", "QMax")); + auto const qStep = stringValueOrEmpty(defaults.getOptionalValue<double>( + "MomentumTransferStep", "dQ/Q")); + auto const maybeScaleFactor = defaults.getOptionalValue<double>("ScaleFactor", "ScaleFactor"); - auto processingInstructions = defaults.getOptionalValue<std::string>( - "ProcessingInstructions", "ProcessingInstructions"); - auto perThetaDefaults = std::vector<PerThetaDefaults>(); - perThetaDefaults.emplace_back(theta, transmissionRuns, qRange, scaleFactor, - processingInstructions); - + auto const scaleFactor = stringValueOrEmpty(maybeScaleFactor); + auto const processingInstructions = defaults.getStringOrEmpty("ProcessingInstructions", + "ProcessingInstructions"); + auto perThetaDefaults = std::vector<std::array<std::string, 8>>(); + perThetaDefaults.emplace_back(std::array<std::string, 8>{ + theta,firstTransmissionRun, secondTransmissionRun, + qMin, qMax, qStep, scaleFactor, processingInstructions}); + auto validate = PerThetaDefaultsTableValidator(); + auto const tolerance = 0.0; // irrelevant because theta is empty + auto perThetaValidationResult = validate(perThetaDefaults, tolerance); + if (!perThetaValidationResult.isValid()) + throw std::invalid_argument("Errors were found in the per-angle default values"); + return Experiment( analysisMode, reductionType, summationType, includePartialBins, debug, std::move(polarizationCorrections), std::move(floodCorrections), std::move(transmissionRunRange), std::move(stitchParameters), - std::move(perThetaDefaults)); + std::move(perThetaValidationResult.assertValid())); } } // namespace CustomInterfaces } // namespace MantidQt diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentPresenter.cpp b/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentPresenter.cpp index 0839b78a6362930bc02bb8d6fd9ddbee0288e739..e7a58b13ff94d40c6aa49c8420b415fbb74786dc 100644 --- a/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentPresenter.cpp +++ b/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/ExperimentPresenter.cpp @@ -255,10 +255,9 @@ void ExperimentPresenter::updateViewFromModel() { if (m_model.transmissionRunRange()) { m_view->setTransmissionStartOverlap(m_model.transmissionRunRange()->min()); m_view->setTransmissionEndOverlap(m_model.transmissionRunRange()->max()); - if (m_model.transmissionRunRange()->isValid(false)) - m_view->showTransmissionRangeValid(); - else - m_view->showTransmissionRangeInvalid(); + } else { + m_view->setTransmissionStartOverlap(0.0); + m_view->setTransmissionEndOverlap(0.0); } m_view->setPolarizationCorrectionType(polarizationCorrectionTypeToString( m_model.polarizationCorrections().correctionType())); @@ -269,9 +268,10 @@ void ExperimentPresenter::updateViewFromModel() { else m_view->setFloodWorkspace(""); m_view->setStitchOptions(m_model.stitchParametersString()); - // The model can't be created with invalid stitch parameters so always set - // them - // to be valid + + // We don't allow invalid config so reset all state to valid + m_view->showAllPerAngleOptionsAsValid(); + m_view->showTransmissionRangeValid(); m_view->showStitchParametersValid(); updateWidgetEnabledState(); diff --git a/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentOptionDefaultsTest.h b/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentOptionDefaultsTest.h new file mode 100644 index 0000000000000000000000000000000000000000..d4543b904bb26e11621bd1c6a09e15a503d983bd --- /dev/null +++ b/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentOptionDefaultsTest.h @@ -0,0 +1,174 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2018 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source +// & Institut Laue - Langevin +// SPDX - License - Identifier: GPL - 3.0 + +#ifndef MANTID_CUSTOMINTERFACES_EXPERIMENTOPTIONDEFAULTSTEST_H_ +#define MANTID_CUSTOMINTERFACES_EXPERIMENTOPTIONDEFAULTSTEST_H_ + +#include "../../../ISISReflectometry/GUI/Experiment/ExperimentOptionDefaults.h" +#include "MantidAPI/FrameworkManager.h" +#include "MantidAPI/MatrixWorkspace.h" +#include "MantidGeometry/Instrument.h" +#include "MantidTestHelpers/ReflectometryHelper.h" + +#include <cxxtest/TestSuite.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> + +using namespace MantidQt::CustomInterfaces; +using testing::AtLeast; +using testing::Mock; +using testing::NiceMock; +using testing::Return; +using testing::_; + +// The missing braces warning is a false positive - +// https://llvm.org/bugs/show_bug.cgi?id=21629 +GNU_DIAG_OFF("missing-braces") + +class ExperimentOptionDefaultsTest : public CxxTest::TestSuite { +public: + // This pair of boilerplate methods prevent the suite being created statically + // This means the constructor isn't called when running other tests + static ExperimentOptionDefaultsTest *createSuite() { + return new ExperimentOptionDefaultsTest(); + } + static void destroySuite(ExperimentOptionDefaultsTest *suite) { delete suite; } + + ExperimentOptionDefaultsTest() : m_view() { + Mantid::API::FrameworkManager::Instance(); + } + + void testValidAnalysisMode() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Analysis"); + presenter.notifyRestoreDefaultsRequested(); + TS_ASSERT_EQUALS(presenter.experiment().analysisMode(), + AnalysisMode::MultiDetector); + verifyAndClear(); + } + + void testInvalidAnalysisMode() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Analysis_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + + void testValidReductionOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Reduction"); + presenter.notifyRestoreDefaultsRequested(); + TS_ASSERT_EQUALS(presenter.experiment().summationType(), + SummationType::SumInQ); + TS_ASSERT_EQUALS(presenter.experiment().reductionType(), + ReductionType::NonFlatSample); + TS_ASSERT_EQUALS(presenter.experiment().includePartialBins(), true); + verifyAndClear(); + } + + void testInvalidReductionOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Reduction_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + + void testValidDebugOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Debug"); + presenter.notifyRestoreDefaultsRequested(); + TS_ASSERT_EQUALS(presenter.experiment().debug(), true); + verifyAndClear(); + } + + void testInvalidDebugOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Debug_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + + void testValidPerThetaOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("PerTheta"); + presenter.notifyRestoreDefaultsRequested(); + auto expected = PerThetaDefaults(boost::none, TransmissionRunPair(), + RangeInQ(0.01, 0.03, 0.2), 0.7, + std::string("390-415")); + TS_ASSERT_EQUALS(presenter.experiment().perThetaDefaults().size(), 1); + TS_ASSERT_EQUALS(presenter.experiment().perThetaDefaults().front(), + expected); + verifyAndClear(); + } + + void testInvalidPerThetaOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("PerTheta_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + + void testValidTransmissionRunRange() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("TransmissionRunRange"); + presenter.notifyRestoreDefaultsRequested(); + auto const expected = RangeInLambda{10.0, 12.0}; + TS_ASSERT_EQUALS(presenter.experiment().transmissionRunRange(), expected); + verifyAndClear(); + } + + void testInvalidTransmissionRunRange() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("TransmissionRunRange_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + + void testValidCorrectionOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Correction"); + presenter.notifyRestoreDefaultsRequested(); + TS_ASSERT_EQUALS( + presenter.experiment().polarizationCorrections().correctionType(), + PolarizationCorrectionType::ParameterFile); + TS_ASSERT_EQUALS(presenter.experiment().floodCorrections().correctionType(), + FloodCorrectionType::ParameterFile); + verifyAndClear(); + } + + void testInvalidCorrectionOptions() { + auto presenter = makePresenter(); + expectInstrumentWithParameters("Correction_Invalid"); + TS_ASSERT_THROWS(presenter.notifyRestoreDefaultsRequested(), std::invalid_argument); + } + +private: + void verifyAndClear() { + TS_ASSERT(Mock::VerifyAndClearExpectations(&m_view)); + TS_ASSERT(Mock::VerifyAndClearExpectations(&m_mainPresenter)); + } + + // Get a dummy reflectometry instrument with the given parameters file type. + // paramsType is appended to "REFL_Parameters_" to form the name for the file + // to load. See ReflectometryHelper.h for details. + void Mantid::Geometry::Instrument_const_sptr + getInstrumentWithParameters(std::string const ¶msType) { + auto workspace = Mantid::TestHelpers::createREFL_WS( + 5, 100.0, 500.0, {1.0, 2.0, 3.0, 4.0, 5.0}, paramsType); + return workspace->getInstrument(); + } + + void expectInstrumentWithDefaultParameters() { + // Use the default REFL_Parameters.xml file, which is empty + expectInstrumentWithParameters(""); + } + + void expectInstrumentWithParameters(std::string const ¶msType) { + // Use the REFL_Parameters_<paramsType> file + auto instrument = getInstrumentWithParameters(paramsType); + EXPECT_CALL(m_mainPresenter, instrument()) + .Times(1) + .WillOnce(Return(instrument)); + } + +GNU_DIAG_ON("missing-braces") + +#endif // MANTID_CUSTOMINTERFACES_EXPERIMENTOPTIONDEFAULTSTEST_H_ diff --git a/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentPresenterTest.h b/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentPresenterTest.h index a2f500df0c292ad06cdda5d8e472916f4075bd90..c89ea611e13d0ba58e14b96be1b6e44861eecec3 100644 --- a/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentPresenterTest.h +++ b/qt/scientific_interfaces/test/ISISReflectometry/Experiment/ExperimentPresenterTest.h @@ -525,6 +525,7 @@ public: expectInstrumentWithParameters("TransmissionRunRange"); EXPECT_CALL(m_view, setTransmissionStartOverlap(10.0)).Times(1); EXPECT_CALL(m_view, setTransmissionEndOverlap(12.0)).Times(1); + EXPECT_CALL(m_view, showTransmissionRangeValid()).Times(1); presenter.notifyRestoreDefaultsRequested(); verifyAndClear(); }