From 65e6c99e116d6dc5290ea10562e4795da7e5a63c Mon Sep 17 00:00:00 2001
From: Jose Borreguero <borreguero@gmail.com>
Date: Tue, 30 Mar 2021 11:45:58 -0400
Subject: [PATCH] implement getSamplePos and setSamplePos in BasePeak (#30962)

IPeak will no longer have knowledge of instrument type information. This is migrated to
the new BasePeak type.
---
 Framework/Crystal/src/PeaksIntersection.cpp   | 13 ++--
 Framework/Crystal/test/AddPeakHKLTest.h       |  5 +-
 .../inc/MantidDataObjects/BasePeak.h          |  7 +++
 .../inc/MantidDataObjects/LeanElasticPeak.h   |  8 ---
 .../DataObjects/inc/MantidDataObjects/Peak.h  | 15 ++---
 .../inc/MantidDataObjects/PeaksWorkspace.h    | 13 ++--
 Framework/DataObjects/src/BasePeak.cpp        | 37 ++++++++++--
 Framework/DataObjects/src/LeanElasticPeak.cpp | 50 ----------------
 Framework/DataObjects/src/Peak.cpp            | 60 ++++++-------------
 .../DataObjects/test/LeanElasticPeakTest.h    | 12 +---
 .../DataObjects/test/PeaksWorkspaceTest.h     |  4 +-
 .../inc/MantidGeometry/Crystal/IPeak.h        |  4 --
 Framework/Geometry/test/MockObjects.h         |  4 --
 .../mantid/api/src/Exports/IPeak.cpp          |  2 -
 .../IntegratePeaksProfileFitting.py           |  2 +-
 .../plugins/algorithms/SaveHKLCW.py           | 14 +++--
 .../test/python/mantid/geometry/IPeakTest.py  |  4 --
 qt/widgets/instrumentview/src/PeakOverlay.cpp | 14 ++++-
 18 files changed, 100 insertions(+), 168 deletions(-)

diff --git a/Framework/Crystal/src/PeaksIntersection.cpp b/Framework/Crystal/src/PeaksIntersection.cpp
index a713553ed21..dd91f53c312 100644
--- a/Framework/Crystal/src/PeaksIntersection.cpp
+++ b/Framework/Crystal/src/PeaksIntersection.cpp
@@ -16,6 +16,7 @@
 using namespace Mantid::API;
 using namespace Mantid::Geometry;
 using namespace Mantid::Kernel;
+using Mantid::DataObjects::Peak;
 using Mantid::DataObjects::PeaksWorkspace;
 using Mantid::DataObjects::PeaksWorkspace_sptr;
 
@@ -78,14 +79,14 @@ void PeaksIntersection::executePeaksIntersection(const bool checkPeakExtents) {
 
   m_peakRadius = this->getProperty("PeakRadius");
 
-  // Find the coordinate frame to use an set up boost function for this.
-  boost::function<V3D(IPeak *)> coordFrameFunc = &IPeak::getHKL;
+  boost::function<V3D(Peak *)> coordFrameFunc;
+  coordFrameFunc = &Peak::getHKL;
   if (coordinateFrame == detectorSpaceFrame()) {
-    coordFrameFunc = &IPeak::getDetectorPosition;
+    coordFrameFunc = &Peak::getDetectorPosition;
   } else if (coordinateFrame == qLabFrame()) {
-    coordFrameFunc = &IPeak::getQLabFrame;
+    coordFrameFunc = &Peak::getQLabFrame;
   } else if (coordinateFrame == qSampleFrame()) {
-    coordFrameFunc = &IPeak::getQSampleFrame;
+    coordFrameFunc = &Peak::getQSampleFrame;
   }
 
   // Create the faces.
@@ -131,7 +132,7 @@ void PeaksIntersection::executePeaksIntersection(const bool checkPeakExtents) {
   PARALLEL_FOR_IF(Kernel::threadSafe(*ws, *outputWorkspace))
   for (int i = 0; i < nPeaks; ++i) {
     PARALLEL_START_INTERUPT_REGION
-    IPeak *peak = ws->getPeakPtr(i);
+    Peak *peak = dynamic_cast<Peak *>(ws->getPeakPtr(i));
     V3D peakCenter = coordFrameFunc(peak);
 
     if (i % frequency == 0)
diff --git a/Framework/Crystal/test/AddPeakHKLTest.h b/Framework/Crystal/test/AddPeakHKLTest.h
index 1caad2ef787..fa719b24239 100644
--- a/Framework/Crystal/test/AddPeakHKLTest.h
+++ b/Framework/Crystal/test/AddPeakHKLTest.h
@@ -96,8 +96,7 @@ public:
     IPeaksWorkspace_sptr ws_out = alg.getProperty("Workspace");
 
     // Get the peak just added.
-    const IPeak &peak = ws_out->getPeak(0);
-
+    auto peak = dynamic_cast<const Peak &>(ws_out->getPeak(0));
     /*
      Now we check we have made a self - consistent peak
      */
@@ -113,7 +112,7 @@ public:
     TSM_ASSERT_EQUALS("This detector id does not match what we expect from the "
                       "instrument definition",
                       1, detector->getID());
-    TSM_ASSERT_EQUALS("Thie detector position is wrong", detectorPos,
+    TSM_ASSERT_EQUALS("This detector position is wrong", detectorPos,
                       detector->getPos());
     TSM_ASSERT_EQUALS("Goniometer has not been set properly", goniometer.getR(),
                       peak.getGoniometerMatrix());
diff --git a/Framework/DataObjects/inc/MantidDataObjects/BasePeak.h b/Framework/DataObjects/inc/MantidDataObjects/BasePeak.h
index 68e1167512b..7cd806d36d3 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/BasePeak.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/BasePeak.h
@@ -75,6 +75,10 @@ public:
   void setIntHKL(const Kernel::V3D &HKL) override;
   void setIntMNP(const Mantid::Kernel::V3D &MNP) override;
 
+  Mantid::Kernel::V3D getSamplePos() const override;
+  void setSamplePos(double samX, double samY, double samZ) override;
+  void setSamplePos(const Mantid::Kernel::V3D &XYZ) override;
+
   double getIntensity() const override;
   double getSigmaIntensity() const override;
   double getIntensityOverSigma() const override;
@@ -121,6 +125,9 @@ protected:
   // ki-kf for Inelastic convention; kf-ki for Crystallography convention
   std::string convention;
 
+  /// Cached sample position
+  Mantid::Kernel::V3D m_samplePos;
+
 private:
   /// Name of the parent bank
   std::string m_bankName;
diff --git a/Framework/DataObjects/inc/MantidDataObjects/LeanElasticPeak.h b/Framework/DataObjects/inc/MantidDataObjects/LeanElasticPeak.h
index f05a958380e..decb5449d98 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/LeanElasticPeak.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/LeanElasticPeak.h
@@ -57,17 +57,11 @@ public:
   // Construct a peak from a reference to the interface
   explicit LeanElasticPeak(const Geometry::IPeak &ipeak);
 
-  Geometry::IDetector_const_sptr getDetector() const override;
   std::shared_ptr<const Geometry::ReferenceFrame>
   getReferenceFrame() const override;
 
-  void setSamplePos(double, double, double) override;
-  void setSamplePos(const Mantid::Kernel::V3D &) override;
-
   Mantid::Kernel::V3D getQLabFrame() const override;
   Mantid::Kernel::V3D getQSampleFrame() const override;
-  Mantid::Kernel::V3D getDetectorPosition() const override;
-  Mantid::Kernel::V3D getDetectorPositionNoCheck() const override;
 
   void setQSampleFrame(const Mantid::Kernel::V3D &QSampleFrame,
                        boost::optional<double> = boost::none) override;
@@ -89,8 +83,6 @@ public:
   void setInitialEnergy(double m_initialEnergy) override;
   void setFinalEnergy(double m_finalEnergy) override;
 
-  virtual Mantid::Kernel::V3D getDetPos() const override;
-  virtual Mantid::Kernel::V3D getSamplePos() const override;
   double getL1() const override;
   double getL2() const override;
 
diff --git a/Framework/DataObjects/inc/MantidDataObjects/Peak.h b/Framework/DataObjects/inc/MantidDataObjects/Peak.h
index cf6f0eb6536..3b34bfed864 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/Peak.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/Peak.h
@@ -85,7 +85,7 @@ public:
   const std::set<int> &getContributingDetIDs() const;
 
   void setInstrument(const Geometry::Instrument_const_sptr &inst);
-  Geometry::IDetector_const_sptr getDetector() const override;
+  Geometry::IDetector_const_sptr getDetector() const;
   Geometry::Instrument_const_sptr getInstrument() const;
   std::shared_ptr<const Geometry::ReferenceFrame>
   getReferenceFrame() const override;
@@ -93,13 +93,10 @@ public:
   bool findDetector();
   bool findDetector(const Geometry::InstrumentRayTracer &tracer);
 
-  void setSamplePos(double samX, double samY, double samZ) override;
-  void setSamplePos(const Mantid::Kernel::V3D &XYZ) override;
-
   Mantid::Kernel::V3D getQLabFrame() const override;
   Mantid::Kernel::V3D getQSampleFrame() const override;
-  Mantid::Kernel::V3D getDetectorPosition() const override;
-  Mantid::Kernel::V3D getDetectorPositionNoCheck() const override;
+  Mantid::Kernel::V3D getDetectorPosition() const;
+  Mantid::Kernel::V3D getDetectorPositionNoCheck() const;
 
   void setQSampleFrame(
       const Mantid::Kernel::V3D &QSampleFrame,
@@ -121,8 +118,7 @@ public:
   void setInitialEnergy(double m_initialEnergy) override;
   void setFinalEnergy(double m_finalEnergy) override;
 
-  virtual Mantid::Kernel::V3D getDetPos() const override;
-  virtual Mantid::Kernel::V3D getSamplePos() const override;
+  virtual Mantid::Kernel::V3D getDetPos() const;
   double getL1() const override;
   double getL2() const override;
 
@@ -155,8 +151,7 @@ private:
 
   /// Cached source position
   Mantid::Kernel::V3D sourcePos;
-  /// Cached sample position
-  Mantid::Kernel::V3D samplePos;
+
   /// Cached detector position
   Mantid::Kernel::V3D detPos;
 
diff --git a/Framework/DataObjects/inc/MantidDataObjects/PeaksWorkspace.h b/Framework/DataObjects/inc/MantidDataObjects/PeaksWorkspace.h
index c277923d6af..07c23c28c2c 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/PeaksWorkspace.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/PeaksWorkspace.h
@@ -15,6 +15,7 @@
 #include "MantidKernel/SpecialCoordinateSystem.h"
 #include "MantidKernel/V3D.h"
 
+using Mantid::Geometry::IPeak_uptr;
 // IsamplePosition should be IsampleOrientation
 namespace Mantid {
 //----------------------------------------------------------------------
@@ -87,24 +88,22 @@ public:
   Peak &getPeak(int peakNum) override;
   const Peak &getPeak(int peakNum) const override;
 
-  std::unique_ptr<Geometry::IPeak> createPeak(
+  IPeak_uptr createPeak(
       const Kernel::V3D &QLabFrame,
       boost::optional<double> detectorDistance = boost::none) const override;
 
-  std::unique_ptr<Geometry::IPeak>
+  IPeak_uptr
   createPeak(const Kernel::V3D &Position,
              const Kernel::SpecialCoordinateSystem &frame) const override;
 
-  std::unique_ptr<Geometry::IPeak>
-  createPeakQSample(const Kernel::V3D &position) const override;
+  IPeak_uptr createPeakQSample(const Kernel::V3D &position) const override;
 
   std::vector<std::pair<std::string, std::string>>
   peakInfo(const Kernel::V3D &qFrame, bool labCoords) const override;
 
-  std::unique_ptr<Geometry::IPeak>
-  createPeakHKL(const Kernel::V3D &HKL) const override;
+  IPeak_uptr createPeakHKL(const Kernel::V3D &HKL) const override;
 
-  std::unique_ptr<Geometry::IPeak> createPeak() const override;
+  IPeak_uptr createPeak() const override;
 
   int peakInfoNumber(const Kernel::V3D &qFrame, bool labCoords) const override;
 
diff --git a/Framework/DataObjects/src/BasePeak.cpp b/Framework/DataObjects/src/BasePeak.cpp
index 8373a048780..b3b4cdc9867 100644
--- a/Framework/DataObjects/src/BasePeak.cpp
+++ b/Framework/DataObjects/src/BasePeak.cpp
@@ -31,8 +31,8 @@ namespace DataObjects {
 //----------------------------------------------------------------------------------------------
 /** Default constructor */
 BasePeak::BasePeak()
-    : m_H(0), m_K(0), m_L(0), m_intensity(0), m_sigmaIntensity(0),
-      m_binCount(0), m_absorptionWeightedPathLength(0),
+    : m_samplePos(V3D(0, 0, 0)), m_H(0), m_K(0), m_L(0), m_intensity(0),
+      m_sigmaIntensity(0), m_binCount(0), m_absorptionWeightedPathLength(0),
       m_GoniometerMatrix(3, 3, true), m_InverseGoniometerMatrix(3, 3, true),
       m_runNumber(0), m_monitorCount(0), m_row(-1), m_col(-1), m_peakNumber(0),
       m_intHKL(V3D(0, 0, 0)), m_intMNP(V3D(0, 0, 0)),
@@ -60,10 +60,10 @@ BasePeak::BasePeak(const Mantid::Kernel::Matrix<double> &goniometer)
 }
 
 BasePeak::BasePeak(const BasePeak &other)
-    : convention(other.convention), m_bankName(other.m_bankName),
-      m_H(other.m_H), m_K(other.m_K), m_L(other.m_L),
-      m_intensity(other.m_intensity), m_sigmaIntensity(other.m_sigmaIntensity),
-      m_binCount(other.m_binCount),
+    : convention(other.convention), m_samplePos(other.m_samplePos),
+      m_bankName(other.m_bankName), m_H(other.m_H), m_K(other.m_K),
+      m_L(other.m_L), m_intensity(other.m_intensity),
+      m_sigmaIntensity(other.m_sigmaIntensity), m_binCount(other.m_binCount),
       m_absorptionWeightedPathLength(other.m_absorptionWeightedPathLength),
       m_GoniometerMatrix(other.m_GoniometerMatrix),
       m_InverseGoniometerMatrix(other.m_InverseGoniometerMatrix),
@@ -192,6 +192,31 @@ void BasePeak::setIntMNP(const V3D &MNP) {
   m_intMNP = V3D(std::round(MNP[0]), std::round(MNP[1]), std::round(MNP[2]));
 }
 
+/** Return the sample position vector */
+Mantid::Kernel::V3D BasePeak::getSamplePos() const { return m_samplePos; }
+
+/** Set sample position
+ *
+ * @ doubles x,y,z-> m_samplePos(x), m_samplePos(y), m_samplePos(z)
+ */
+void BasePeak::setSamplePos(double samX, double samY, double samZ) {
+
+  this->m_samplePos[0] = samX;
+  this->m_samplePos[1] = samY;
+  this->m_samplePos[2] = samZ;
+}
+
+/** Set sample position
+ *
+ * @param XYZ :: vector x,y,z-> m_samplePos(x), m_samplePos(y), m_samplePos(z)
+ */
+void BasePeak::setSamplePos(const Mantid::Kernel::V3D &XYZ) {
+
+  this->m_samplePos[0] = XYZ[0];
+  this->m_samplePos[1] = XYZ[1];
+  this->m_samplePos[2] = XYZ[2];
+}
+
 //----------------------------------------------------------------------------------------------
 /** Return the # of counts in the bin at its peak*/
 double BasePeak::getBinCount() const { return m_binCount; }
diff --git a/Framework/DataObjects/src/LeanElasticPeak.cpp b/Framework/DataObjects/src/LeanElasticPeak.cpp
index 5c100f1b2d2..0864a4617f8 100644
--- a/Framework/DataObjects/src/LeanElasticPeak.cpp
+++ b/Framework/DataObjects/src/LeanElasticPeak.cpp
@@ -99,13 +99,6 @@ void LeanElasticPeak::setWavelength(double wavelength) {
   m_wavelength = wavelength;
 }
 
-//----------------------------------------------------------------------------------------------
-/** Return a shared ptr to the detector at center of peak. */
-Geometry::IDetector_const_sptr LeanElasticPeak::getDetector() const {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak::getDetector(): Has no detector ID");
-}
-
 /** Return a shared ptr to the reference frame for this peak. */
 std::shared_ptr<const Geometry::ReferenceFrame>
 LeanElasticPeak::getReferenceFrame() const {
@@ -241,18 +234,6 @@ double LeanElasticPeak::getInitialEnergy() const { return getFinalEnergy(); }
  * elastic so always 0 */
 double LeanElasticPeak::getEnergyTransfer() const { return 0.; }
 
-/** Set sample position */
-void LeanElasticPeak::setSamplePos(double, double, double) {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no sample information");
-}
-
-/** Set sample position  */
-void LeanElasticPeak::setSamplePos(const Mantid::Kernel::V3D &) {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no sample information");
-}
-
 /** Set the final energy */
 void LeanElasticPeak::setFinalEnergy(double) {
   throw Exception::NotImplementedError("Use LeanElasticPeak::setWavelength");
@@ -263,20 +244,6 @@ void LeanElasticPeak::setInitialEnergy(double) {
   throw Exception::NotImplementedError("Use LeanElasticPeak::setWavelength");
 }
 
-// -------------------------------------------------------------------------------------
-/** Return the detector position vector */
-Mantid::Kernel::V3D LeanElasticPeak::getDetPos() const {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no detector information");
-}
-
-// -------------------------------------------------------------------------------------
-/** Return the sample position vector */
-Mantid::Kernel::V3D LeanElasticPeak::getSamplePos() const {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no sample information");
-}
-
 // -------------------------------------------------------------------------------------
 /** Return the L1 flight path length (source to sample), in meters. */
 double LeanElasticPeak::getL1() const {
@@ -305,23 +272,6 @@ LeanElasticPeak &LeanElasticPeak::operator=(const LeanElasticPeak &other) {
   return *this;
 }
 
-/**
- Forwarding function. Exposes the detector position directly.
- */
-Mantid::Kernel::V3D LeanElasticPeak::getDetectorPositionNoCheck() const {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no detector information");
-}
-
-/**
- Forwarding function. Exposes the detector position directly, but checks that
- the detector is not null before accessing its position. Throws if null.
- */
-Mantid::Kernel::V3D LeanElasticPeak::getDetectorPosition() const {
-  throw Exception::NotImplementedError(
-      "LeanElasticPeak has no detector information");
-}
-
 Mantid::Kernel::Logger LeanElasticPeak::g_log("PeakLogger");
 
 } // namespace DataObjects
diff --git a/Framework/DataObjects/src/Peak.cpp b/Framework/DataObjects/src/Peak.cpp
index 0f6107e62b0..c32270b7ce4 100644
--- a/Framework/DataObjects/src/Peak.cpp
+++ b/Framework/DataObjects/src/Peak.cpp
@@ -155,13 +155,13 @@ Peak::Peak(const Peak &other)
     : BasePeak(other), m_inst(other.m_inst), m_det(other.m_det),
       m_detectorID(other.m_detectorID), m_initialEnergy(other.m_initialEnergy),
       m_finalEnergy(other.m_finalEnergy), sourcePos(other.sourcePos),
-      samplePos(other.samplePos), detPos(other.detPos),
-      m_detIDs(other.m_detIDs) {}
+      detPos(other.detPos), m_detIDs(other.m_detIDs) {}
 
 //----------------------------------------------------------------------------------------------
 /** Constructor making a Peak from IPeak interface
  *
- * @param ipeak :: const reference to an IPeak object
+ * @param ipeak :: const reference to an IPeak object though actually
+ * referencing a Peak object.
  * @return
  */
 Peak::Peak(const Geometry::IPeak &ipeak)
@@ -320,7 +320,7 @@ void Peak::setInstrument(const Geometry::Instrument_const_sptr &inst) {
                                                "instrument");
 
   sourcePos = sourceObj->getPos();
-  samplePos = sampleObj->getPos();
+  m_samplePos = sampleObj->getPos();
 }
 
 //----------------------------------------------------------------------------------------------
@@ -377,8 +377,8 @@ double Peak::getTOF() const {
 /** Calculate the scattering angle of the peak  */
 double Peak::getScattering() const {
   // The detector is at 2 theta scattering angle
-  V3D beamDir = samplePos - sourcePos;
-  V3D detDir = detPos - samplePos;
+  V3D beamDir = m_samplePos - sourcePos;
+  V3D detDir = detPos - m_samplePos;
 
   return detDir.angle(beamDir);
 }
@@ -387,7 +387,7 @@ double Peak::getScattering() const {
 /** Calculate the azimuthal angle of the peak  */
 double Peak::getAzimuthal() const {
   // The detector is at 2 theta scattering angle
-  V3D detDir = detPos - samplePos;
+  V3D detDir = detPos - m_samplePos;
 
   return atan2(detDir.Y(), detDir.X());
 }
@@ -396,8 +396,8 @@ double Peak::getAzimuthal() const {
 /** Calculate the d-spacing of the peak, in 1/Angstroms  */
 double Peak::getDSpacing() const {
   // The detector is at 2 theta scattering angle
-  V3D beamDir = samplePos - sourcePos;
-  V3D detDir = detPos - samplePos;
+  V3D beamDir = m_samplePos - sourcePos;
+  V3D detDir = detPos - m_samplePos;
 
   double two_theta;
   try {
@@ -423,10 +423,10 @@ double Peak::getDSpacing() const {
  * */
 Mantid::Kernel::V3D Peak::getQLabFrame() const {
   // Normalized beam direction
-  V3D beamDir = samplePos - sourcePos;
+  V3D beamDir = m_samplePos - sourcePos;
   beamDir /= beamDir.norm();
   // Normalized detector direction
-  V3D detDir = (detPos - samplePos);
+  V3D detDir = (detPos - m_samplePos);
   detDir /= detDir.norm();
 
   // Energy in J of the neutron
@@ -550,7 +550,7 @@ void Peak::setQLabFrame(const Mantid::Kernel::V3D &qLab,
 
   // Use the given detector distance to find the detector position.
   if (detectorDistance.is_initialized()) {
-    detPos = samplePos + detectorDir * detectorDistance.get();
+    detPos = m_samplePos + detectorDir * detectorDistance.get();
     // We do not-update the detector as by manually setting the distance the
     // client seems to know better.
   } else {
@@ -576,7 +576,7 @@ V3D Peak::getVirtualDetectorPosition(const V3D &detectorDir) const {
   }
   const auto object = std::dynamic_pointer_cast<const ObjComponent>(component);
   const auto distance =
-      object->shape()->distance(Geometry::Track(samplePos, detectorDir));
+      object->shape()->distance(Geometry::Track(m_samplePos, detectorDir));
   return detectorDir * distance;
 }
 
@@ -612,7 +612,7 @@ bool Peak::findDetector() {
  */
 bool Peak::findDetector(const InstrumentRayTracer &tracer) {
   // Scattered beam direction
-  const V3D beam = normalize(detPos - samplePos);
+  const V3D beam = normalize(detPos - m_samplePos);
 
   return findDetector(beam, tracer);
 }
@@ -680,28 +680,6 @@ double Peak::getEnergyTransfer() const {
   return getInitialEnergy() - getFinalEnergy();
 }
 
-/** Set sample position
- *
- * @ doubles x,y,z-> samplePos(x), samplePos(y), samplePos(z)
- */
-void Peak::setSamplePos(double samX, double samY, double samZ) {
-
-  this->samplePos[0] = samX;
-  this->samplePos[1] = samY;
-  this->samplePos[2] = samZ;
-}
-
-/** Set sample position
- *
- * @param XYZ :: vector x,y,z-> samplePos(x), samplePos(y), samplePos(z)
- */
-void Peak::setSamplePos(const Mantid::Kernel::V3D &XYZ) {
-
-  this->samplePos[0] = XYZ[0];
-  this->samplePos[1] = XYZ[1];
-  this->samplePos[2] = XYZ[2];
-}
-
 /** Set the final energy
  * @param m_finalEnergy :: final energy in meV   */
 void Peak::setFinalEnergy(double m_finalEnergy) {
@@ -718,17 +696,13 @@ void Peak::setInitialEnergy(double m_initialEnergy) {
 /** Return the detector position vector */
 Mantid::Kernel::V3D Peak::getDetPos() const { return detPos; }
 
-// -------------------------------------------------------------------------------------
-/** Return the sample position vector */
-Mantid::Kernel::V3D Peak::getSamplePos() const { return samplePos; }
-
 // -------------------------------------------------------------------------------------
 /** Return the L1 flight path length (source to sample), in meters. */
-double Peak::getL1() const { return (samplePos - sourcePos).norm(); }
+double Peak::getL1() const { return (m_samplePos - sourcePos).norm(); }
 
 // -------------------------------------------------------------------------------------
 /** Return the L2 flight path length (sample to detector), in meters. */
-double Peak::getL2() const { return (detPos - samplePos).norm(); }
+double Peak::getL2() const { return (detPos - m_samplePos).norm(); }
 
 /**
  * @brief Assignement operator overload
@@ -744,7 +718,7 @@ Peak &Peak::operator=(const Peak &other) {
     m_initialEnergy = other.m_initialEnergy;
     m_finalEnergy = other.m_finalEnergy;
     sourcePos = other.sourcePos;
-    samplePos = other.samplePos;
+    m_samplePos = other.m_samplePos;
     detPos = other.detPos;
     m_detIDs = other.m_detIDs;
   }
diff --git a/Framework/DataObjects/test/LeanElasticPeakTest.h b/Framework/DataObjects/test/LeanElasticPeakTest.h
index d1a50ae394c..23c0e512d36 100644
--- a/Framework/DataObjects/test/LeanElasticPeakTest.h
+++ b/Framework/DataObjects/test/LeanElasticPeakTest.h
@@ -37,14 +37,7 @@ public:
     TS_ASSERT(std::isinf(p.getFinalEnergy()))
     TS_ASSERT_EQUALS(p.getQSampleFrame(), V3D(0, 0, 0))
     TS_ASSERT_EQUALS(p.getQLabFrame(), V3D())
-
-    TS_ASSERT_THROWS(p.getDetector(), const Exception::NotImplementedError &)
-    TS_ASSERT_THROWS(p.getDetectorPosition(),
-                     const Exception::NotImplementedError &)
-    TS_ASSERT_THROWS(p.getDetectorPositionNoCheck(),
-                     const Exception::NotImplementedError &)
-    TS_ASSERT_THROWS(p.getDetPos(), const Exception::NotImplementedError &)
-    TS_ASSERT_THROWS(p.getSamplePos(), const Exception::NotImplementedError &)
+    TS_ASSERT_EQUALS(p.getSamplePos(), V3D(0, 0, 0))
     TS_ASSERT_THROWS(p.getTOF(), const Exception::NotImplementedError &)
     TS_ASSERT_EQUALS(p.getScattering(), 0.)
     TS_ASSERT_EQUALS(p.getAzimuthal(), -M_PI)
@@ -330,8 +323,5 @@ public:
 
     TS_ASSERT_EQUALS(leanpeak.getBinCount(), peak.getBinCount());
     TS_ASSERT_EQUALS(leanpeak.getBinCount(), 90);
-
-    TS_ASSERT_THROWS(leanpeak.getDetector(),
-                     const Exception::NotImplementedError &)
   }
 };
diff --git a/Framework/DataObjects/test/PeaksWorkspaceTest.h b/Framework/DataObjects/test/PeaksWorkspaceTest.h
index 58c833c3a32..3d11510620a 100644
--- a/Framework/DataObjects/test/PeaksWorkspaceTest.h
+++ b/Framework/DataObjects/test/PeaksWorkspaceTest.h
@@ -422,8 +422,8 @@ public:
     const auto params = makePeakParameters();
     auto ws = makeWorkspace(params);
     // Create the peak
-    auto peak = ws->createPeakHKL(params.hkl);
-
+    IPeak_uptr ipeak = ws->createPeakHKL(params.hkl);
+    Peak_uptr peak(static_cast<Peak *>(ipeak.release()));
     /*
      Now we check we have made a self - consistent peak
      */
diff --git a/Framework/Geometry/inc/MantidGeometry/Crystal/IPeak.h b/Framework/Geometry/inc/MantidGeometry/Crystal/IPeak.h
index d1ad3dce179..341c3d9873a 100644
--- a/Framework/Geometry/inc/MantidGeometry/Crystal/IPeak.h
+++ b/Framework/Geometry/inc/MantidGeometry/Crystal/IPeak.h
@@ -26,7 +26,6 @@ class InstrumentRayTracer;
 class MANTID_GEOMETRY_DLL IPeak {
 public:
   virtual ~IPeak() = default;
-  virtual Geometry::IDetector_const_sptr getDetector() const = 0;
   virtual std::shared_ptr<const Geometry::ReferenceFrame>
   getReferenceFrame() const = 0;
 
@@ -51,8 +50,6 @@ public:
   virtual void setSamplePos(double samX, double samY, double samZ) = 0;
   virtual void setSamplePos(const Mantid::Kernel::V3D &XYZ) = 0;
   virtual Mantid::Kernel::V3D getSamplePos() const = 0;
-  virtual Mantid::Kernel::V3D getDetectorPosition() const = 0;
-  virtual Mantid::Kernel::V3D getDetectorPositionNoCheck() const = 0;
 
   virtual Mantid::Kernel::V3D getQLabFrame() const = 0;
   virtual Mantid::Kernel::V3D getQSampleFrame() const = 0;
@@ -97,7 +94,6 @@ public:
   virtual int getRow() const = 0;
   virtual int getCol() const = 0;
 
-  virtual Mantid::Kernel::V3D getDetPos() const = 0;
   virtual double getL1() const = 0;
   virtual double getL2() const = 0;
 
diff --git a/Framework/Geometry/test/MockObjects.h b/Framework/Geometry/test/MockObjects.h
index 16aa90434cf..33924160b20 100644
--- a/Framework/Geometry/test/MockObjects.h
+++ b/Framework/Geometry/test/MockObjects.h
@@ -64,7 +64,6 @@ Mock IPeak
 ------------------------------------------------------------*/
 class MockIPeak : public Mantid::Geometry::IPeak {
 public:
-  MOCK_CONST_METHOD0(getDetector, Geometry::IDetector_const_sptr());
   MOCK_CONST_METHOD0(getReferenceFrame,
                      std::shared_ptr<const Geometry::ReferenceFrame>());
   MOCK_CONST_METHOD0(getRunNumber, int());
@@ -123,11 +122,8 @@ public:
   MOCK_CONST_METHOD0(getBankName, std::string());
   MOCK_CONST_METHOD0(getRow, int());
   MOCK_CONST_METHOD0(getCol, int());
-  MOCK_CONST_METHOD0(getDetPos, Mantid::Kernel::V3D());
   MOCK_CONST_METHOD0(getL1, double());
   MOCK_CONST_METHOD0(getL2, double());
-  MOCK_CONST_METHOD0(getDetectorPosition, Mantid::Kernel::V3D());
-  MOCK_CONST_METHOD0(getDetectorPositionNoCheck, Mantid::Kernel::V3D());
   MOCK_CONST_METHOD0(getPeakShape, const Mantid::Geometry::PeakShape &());
   MOCK_METHOD1(setPeakShape, void(Mantid::Geometry::PeakShape *shape));
   MOCK_METHOD1(setPeakShape,
diff --git a/Framework/PythonInterface/mantid/api/src/Exports/IPeak.cpp b/Framework/PythonInterface/mantid/api/src/Exports/IPeak.cpp
index d9398e8d6d9..03e4be767ee 100644
--- a/Framework/PythonInterface/mantid/api/src/Exports/IPeak.cpp
+++ b/Framework/PythonInterface/mantid/api/src/Exports/IPeak.cpp
@@ -209,8 +209,6 @@ void export_IPeak() {
            "For :class:`~mantid.geometry.RectangularDetector` s only, returns "
            "the column (x) of the pixel of the "
            ":class:`~mantid.geometry.Detector`.")
-      .def("getDetPos", &IPeak::getDetPos, arg("self"),
-           "Return the :class:`~mantid.geometry.Detector` position vector")
       .def("getL1", &IPeak::getL1, arg("self"),
            "Return the L1 flight path length (source to "
            ":class:`~mantid.api.Sample`), in meters. ")
diff --git a/Framework/PythonInterface/plugins/algorithms/IntegratePeaksProfileFitting.py b/Framework/PythonInterface/plugins/algorithms/IntegratePeaksProfileFitting.py
index d5bcffc2f38..0635480750f 100644
--- a/Framework/PythonInterface/plugins/algorithms/IntegratePeaksProfileFitting.py
+++ b/Framework/PythonInterface/plugins/algorithms/IntegratePeaksProfileFitting.py
@@ -254,7 +254,7 @@ class IntegratePeaksProfileFitting(PythonAlgorithm):
                     strongPeakParamsToSend = strongPeakParams
 
                 # Will allow forced weak and edge peaks to be fit using a neighboring peak profile
-                Y3D, goodIDX, pp_lambda, params = BVGFT.get3DPeak(peak, peaks_ws, box, padeCoefficients,qMask,
+                Y3D, goodIDX, pp_lambda, params = BVGFT.get3DPeak(peak, peaks_ws, box, padeCoefficients, qMask,
                                                                   nTheta=nTheta, nPhi=nPhi, plotResults=False,
                                                                   zBG=zBG,fracBoxToHistogram=1.0,bgPolyOrder=1,
                                                                   strongPeakParams=strongPeakParamsToSend,
diff --git a/Framework/PythonInterface/plugins/algorithms/SaveHKLCW.py b/Framework/PythonInterface/plugins/algorithms/SaveHKLCW.py
index 85eca56ccbe..62453bc4be8 100644
--- a/Framework/PythonInterface/plugins/algorithms/SaveHKLCW.py
+++ b/Framework/PythonInterface/plugins/algorithms/SaveHKLCW.py
@@ -71,17 +71,19 @@ class SaveHKLCW(PythonAlgorithm):
             if directionCosines:
                 U = peak_ws.sample().getOrientedLattice().getU()
                 sample_pos = peak_ws.getInstrument().getSample().getPos()
-                q_reverse_incident = peak_ws.getInstrument().getSource().getPos() - sample_pos
-                q_reverse_incident = np.array(q_reverse_incident) / q_reverse_incident.norm()
+                source_pos = peak_ws.getInstrument().getSource().getPos()
+                ki_n = sample_pos - source_pos  # direction of incident wavevector
+                ki_n = ki_n * (1. / ki_n.norm())
 
             for p in peak_ws:
                 if directionCosines:
                     R = p.getGoniometerMatrix()
                     RU = np.dot(R, U)
-                    q_diffracted = p.getDetPos() - sample_pos
-                    q_diffracted = np.array(q_diffracted) / q_diffracted.norm()
-                    dir_cos_1 = np.dot(RU.T, q_reverse_incident)
-                    dir_cos_2 = np.dot(RU.T, q_diffracted)
+                    ki = ki_n * (2 * np.pi / p.getWavelength())
+                    kf_n = ki - p.getQLabFrame()  # direction of scattered wavevector
+                    kf_n = kf_n * (1. / kf_n.norm())
+                    dir_cos_1 = np.dot(RU.T, -ki_n)  # notice ki direction is reversed
+                    dir_cos_2 = np.dot(RU.T, kf_n)
                     f.write(
                         "{:4.0f}{:4.0f}{:4.0f}{:8.2f}{:8.2f}{:4d}{:8.5f}{:8.5f}{:8.5f}{:8.5f}{:8.5f}{:8.5f}\n"
                         .format(p.getH(), p.getK(), p.getL(), p.getIntensity(),
diff --git a/Framework/PythonInterface/test/python/mantid/geometry/IPeakTest.py b/Framework/PythonInterface/test/python/mantid/geometry/IPeakTest.py
index 43b6f2d8009..5cbba41e541 100644
--- a/Framework/PythonInterface/test/python/mantid/geometry/IPeakTest.py
+++ b/Framework/PythonInterface/test/python/mantid/geometry/IPeakTest.py
@@ -152,10 +152,6 @@ class IPeakTest(unittest.TestCase):
         self.assertEqual(self._peak.getRow(), row)
         self.assertEqual(self._peak.getCol(), col)
 
-    def test_get_det_pos(self):
-        expected_det_pos = np.array([0.05962, -0.09450, -0.23786])
-        npt.assert_allclose(self._peak.getDetPos(), expected_det_pos, atol=1e-5)
-
     def test_get_l1(self):
         expected_l1 = 8.3
         self.assertEqual(self._peak.getL1(), expected_l1)
diff --git a/qt/widgets/instrumentview/src/PeakOverlay.cpp b/qt/widgets/instrumentview/src/PeakOverlay.cpp
index aa10501faed..5ae8b426095 100644
--- a/qt/widgets/instrumentview/src/PeakOverlay.cpp
+++ b/qt/widgets/instrumentview/src/PeakOverlay.cpp
@@ -7,6 +7,7 @@
 #include "MantidQtWidgets/InstrumentView/PeakOverlay.h"
 #include "MantidAPI/AlgorithmManager.h"
 #include "MantidAPI/IPeaksWorkspace.h"
+#include "MantidDataObjects/Peak.h"
 #include "MantidQtWidgets/InstrumentView/UnwrappedSurface.h"
 
 #include <QList>
@@ -15,6 +16,10 @@
 #include <cmath>
 #include <stdexcept>
 
+namespace {
+Mantid::Kernel::Logger g_log("PeakOverlay");
+}
+
 namespace MantidQt {
 namespace MantidWidgets {
 
@@ -243,7 +248,14 @@ void PeakOverlay::createMarkers(const PeakMarker2D::Style &style) {
   this->clear();
   for (int i = 0; i < nPeaks; ++i) {
     Mantid::Geometry::IPeak &peak = getPeak(i);
-    const Mantid::Kernel::V3D &pos = peak.getDetPos();
+    Mantid::Kernel::V3D pos;
+    try {
+      auto peakFull = dynamic_cast<Mantid::DataObjects::Peak &>(peak);
+      pos = peakFull.getDetPos();
+    } catch (std::bad_cast &) {
+      g_log.error("Cannot create markers for this type of peak");
+      return;
+    }
     // Project the peak (detector) position onto u,v coords
     double u, v, uscale, vscale;
     m_surface->project(pos, u, v, uscale, vscale);
-- 
GitLab