Merge remote-tracking branch 'origin/feature/6192_remove_spectrum_no_…

…duplication' Refs #6192

Conflicts:
	Code/Mantid/Framework/PythonInterface/mantid/api/src/Exports/Axis.cpp

Code/Mantid/Framework/API/inc/MantidAPI/Axis.h

@@ -50,9 +50,9 @@ class MANTID_API_DLL Axis
   virtual ~Axis();
 
   /// Virtual constructor
-  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace = NULL) = 0;
+  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace) = 0;
   /// Virtual constructor for axis of different length
-  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace = NULL) = 0;
+  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace) = 0;
 
   const std::string& title() const;
   std::string& title();
@@ -84,8 +84,8 @@ class MANTID_API_DLL Axis
   /// @param index :: The index
   /// @param value :: The new value
   virtual void setValue(const std::size_t& index, const double& value) = 0;
-  /// Get a non-mutable spectrum index
-  virtual const specid_t& spectraNo(const std::size_t& index) const;
+  /// Get the spectrum index
+  virtual specid_t spectraNo(const std::size_t& index) const;
 
   /// Get the length of the axis
   virtual std::size_t length() const = 0;

Code/Mantid/Framework/API/inc/MantidAPI/NumericAxis.h

@@ -51,8 +51,8 @@ class MANTID_API_DLL NumericAxis: public Axis
 public:
   NumericAxis(const std::size_t& length);
   virtual ~NumericAxis(){}
-  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace = NULL);
-  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace = NULL);
+  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace);
+  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace);
   ///Is the axis numeric - always true for this class
   virtual bool isNumeric() const{return true;}
   virtual std::size_t length() const{return m_values.size();}

Code/Mantid/Framework/API/inc/MantidAPI/SpectraAxis.h

@@ -58,35 +58,35 @@ class MatrixWorkspace;
 class MANTID_API_DLL SpectraAxis: public Axis
 {
 public:
-  explicit SpectraAxis(const std::size_t& length, const bool initWithDefaults = true);
-  explicit SpectraAxis(const std::size_t length, const Geometry::ISpectraDetectorMap & spectramap);
+  explicit SpectraAxis(const MatrixWorkspace* const parentWorkspace);
   virtual ~SpectraAxis(){}
-  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace = NULL);
-  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace = NULL);
-  virtual std::size_t length() const{return m_values.size();}
+  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace);
+  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace);
+  virtual std::size_t length() const;
   /// If this is a spectra Axis - always true for this class
   virtual bool isSpectra() const{return true;}
   virtual double operator()(const std::size_t& index, const std::size_t& verticalIndex = 0) const;
   virtual void setValue(const std::size_t& index, const double& value);
   virtual bool operator==(const Axis&) const;
   std::string label(const std::size_t& index)const;
 
-  const specid_t& spectraNo(const std::size_t& index) const;
+  specid_t spectraNo(const std::size_t& index) const;
   // Get a map that contains the spectra index as the key and the index in the array as teh value
   void getSpectraIndexMap(spec2index_map&) const;
   void getIndexSpectraMap(index2spec_map& map) const;
 
-  /// returns min value defined on axis
-  double getMin()const{return double(m_values.front()) ; }
-   /// returns max value defined on axis
-  double getMax()const{return double(m_values.back()); }
+  double getMin() const;
+  double getMax() const;
+
 private:
   /// Default constructor
   SpectraAxis();
+  /// Private, undefined copy constructor
+  SpectraAxis(const SpectraAxis&);
   /// Private, undefined copy assignment operator
   const SpectraAxis& operator=(const SpectraAxis&);
-  /// A vector holding the axis values for the axis.
-  std::vector<specid_t> m_values;
+  /// A pointer to the workspace holding the axis
+  const MatrixWorkspace* const m_parentWS;
 };
 
 } // namespace API

Code/Mantid/Framework/API/inc/MantidAPI/TextAxis.h

@@ -52,8 +52,8 @@ class MANTID_API_DLL TextAxis: public Axis
 public:
   TextAxis(const std::size_t& length);
   virtual ~TextAxis(){}
-  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace = NULL);
-  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace = NULL);
+  virtual Axis* clone(const MatrixWorkspace* const parentWorkspace);
+  virtual Axis* clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace);
   virtual std::size_t length() const{return m_values.size();}
   /// If this is a TextAxis, always return true for this class
   virtual bool isText() const{return true;}

Code/Mantid/Framework/API/src/Axis.cpp

@@ -83,7 +83,7 @@ double Axis::getValue(const std::size_t& index, const std::size_t& verticalIndex
  *  @return The spectrum number as an int
  *  @throw  domain_error If this method is called on a numeric axis
  */
-const specid_t& Axis::spectraNo(const std::size_t& index) const
+specid_t Axis::spectraNo(const std::size_t& index) const
 {
   UNUSED_ARG(index)
   throw std::domain_error("Cannot call spectraNo() on a non-spectra axis.");

Code/Mantid/Framework/API/src/MatrixWorkspace.cpp

@@ -177,20 +177,10 @@ namespace Mantid
     */
     void MatrixWorkspace::updateSpectraUsingMap()
     {
-      // The Axis1 needs to be set correctly for the ISpectraDetectorMap to make any sense
-      if (m_axes.size() < 2) throw std::runtime_error("MatrixWorkspace::updateSpectraUsingMap() needs a SpectraAxis at index 1 to work.");
-      SpectraAxis * ax1 = dynamic_cast<SpectraAxis *>(m_axes[1]);
-      if (!ax1)
-        throw std::runtime_error("MatrixWorkspace::updateSpectraUsingMap() needs a SpectraAxis at index 1 to work.");
-      if (ax1->length() < this->getNumberHistograms())
-        throw std::runtime_error("MatrixWorkspace::updateSpectraUsingMap(): the SpectraAxis is shorter than the number of histograms! Cannot run.");
-
       for (size_t wi=0; wi < this->getNumberHistograms(); wi++)
       {
-        specid_t specNo = ax1->spectraNo(wi);
-
         ISpectrum * spec = getSpectrum(wi);
-        spec->setSpectrumNo(specNo);
+        specid_t specNo = spec->getSpectrumNo();
 
         std::vector<detid_t> dets = m_spectraMap->getDetectors(specNo);
         spec->clearDetectorIDs();
@@ -221,12 +211,12 @@ namespace Mantid
       if( m_axes.size() > 1 && m_axes[1]->isSpectra() )
       {
         delete m_axes[1];
-        m_axes[1] = new SpectraAxis(pixelIDs.size(), false);
+        m_axes[1] = new SpectraAxis(this);
       }
       else
       {
         if (m_axes.size() == 0) m_axes.push_back( new NumericAxis(this->blocksize()) );
-        m_axes.push_back(  new SpectraAxis(pixelIDs.size(), false) );
+        m_axes.push_back(  new SpectraAxis(this) );
       }
 
       try
@@ -243,9 +233,6 @@ namespace Mantid
           // We keep the entry in the spectraDetectorMap. TODO: Deprecate spectraDetectorMap entirely.
           spectramap->addSpectrumEntry(specNo, detId);
 
-          // Also set the spectrum number in the axis(1). TODO: Remove this, it is redundant (but it is stuck everywhere)
-          m_axes[1]->setValue(index, specNo);
-
           if (index < this->getNumberHistograms())
           {
             ISpectrum * spec = getSpectrum(index);
@@ -287,12 +274,11 @@ namespace Mantid
       if( m_axes.size() > 1 )
       {
         delete m_axes[1];
-        m_axes[1] = new SpectraAxis(this->getNumberHistograms(), false);
+        m_axes[1] = new SpectraAxis(this);
       }
       else
-        m_axes.push_back(  new SpectraAxis(this->getNumberHistograms(), false) );
+        m_axes.push_back(  new SpectraAxis(this) );
 
-      API::Axis *ax1 = getAxis(1);
       API::SpectraDetectorMap *newMap = new API::SpectraDetectorMap;
 
       //Go through all the spectra
@@ -301,7 +287,6 @@ namespace Mantid
         ISpectrum * spec = getSpectrum(wi);
         specid_t specNo = spec->getSpectrumNo();
         newMap->addSpectrumEntries(specNo, spec->getDetectorIDs());
-        ax1->setValue(wi, specNo);
         //std::cout << "generateSpectraMap : wi " << wi << " specNo " << specNo << " detID " << *spec->getDetectorIDs().begin() << std::endl;
       }
 

Code/Mantid/Framework/API/src/SpectraAxis.cpp

@@ -2,6 +2,7 @@
 // Includes
 //----------------------------------------------------------------------
 #include "MantidAPI/SpectraAxis.h"
+#include "MantidAPI/MatrixWorkspace.h"
 #include "MantidKernel/MultiThreaded.h"
 #include "MantidKernel/Exception.h"
 #include "MantidGeometry/ISpectraDetectorMap.h"
@@ -16,82 +17,42 @@ namespace API
 
 using std::size_t;
 
-/**
- * Constructor taking a length and optional flag for initialization
- * @param length :: The length of the axis
- * @param initWithDefaults :: If true the axis values will be initialized 
- * with values from 1->length
+/** Virtual constructor
+ *  @param parentWorkspace The workspace to which this axis belongs
  */
-SpectraAxis::SpectraAxis(const std::size_t& length, const bool initWithDefaults ): Axis()
+SpectraAxis::SpectraAxis(const MatrixWorkspace* const parentWorkspace)
+  : Axis(), m_parentWS(parentWorkspace)
 {
-  m_values.resize(length);
-  if( initWithDefaults )
-  {
-    // For small axes there is no point in the additional thread overhead
-    PARALLEL_FOR_IF((length > 1000)) 
-    for(specid_t i = 0; i < specid_t(length); ++i)
-    {
-      m_values[i] = i + 1;
-    }
-  }
 }
 
-/**
- * Constructor taking a reference to an ISpectraDetectorMap implementation. The
- * axis is initialized to first length unique spectra values provided by the map
- * @param length :: The length of the axis
- * @param spectramap :: A reference to an ISpectraDetectorMap implementation.
+/** Virtual constructor
+ *  @param parentWorkspace The workspace to which the cloned axis belongs
+ *  @return A pointer to a copy of the SpectraAxis on which the method is called
  */
-SpectraAxis::SpectraAxis(const std::size_t length, const Geometry::ISpectraDetectorMap & spectramap) :
-  Axis()
+Axis* SpectraAxis::clone(const MatrixWorkspace* const parentWorkspace)
 {
-  m_values.resize(length);
-  if( length == 0 ) return;
-  Geometry::ISpectraDetectorMap::const_iterator itr = spectramap.cbegin();
-  Geometry::ISpectraDetectorMap::const_iterator iend = spectramap.cend();
-  if( itr == iend )
-  {
-      m_values.resize(0);
-      return;
-  }
-  // it->first = the spectrum number
-  specid_t previous = itr->first;
-  m_values[0] = previous;
-  ++itr;
-  size_t index(1);
-  for(; itr != iend; ++itr)
-  {
-    if( index == length ) break;
-    const specid_t current = itr->first;
-    if( current != previous )
-    {
-      // the spectrum number (in the iterator) just changed.
-      // save this new spectrum number in the SpectraAxis
-      m_values[index] = current;
-      previous = current;
-      // go to the next workspace index
-      ++index;
-    }
-  }
+  SpectraAxis * newAxis = new SpectraAxis(parentWorkspace);
+  // A couple of base class members need copying over manually
+  newAxis->title() = this->title();
+  newAxis->unit() = this->unit();
+  return newAxis;
 }
 
 /** Virtual constructor
- *  @param parentWorkspace :: not used in this implementation
+ *  @param length Not used in this implementation
+ *  @param parentWorkspace The workspace to which the cloned axis belongs
  *  @return A pointer to a copy of the SpectraAxis on which the method is called
  */
-Axis* SpectraAxis::clone(const MatrixWorkspace* const parentWorkspace)
+Axis* SpectraAxis::clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace)
 {
-  (void) parentWorkspace; //Avoid compiler warning
-  return new SpectraAxis(*this);
+  UNUSED_ARG(length)
+  // In this implementation, there's no difference between the clone methods - call the other one
+  return clone(parentWorkspace);
 }
 
-Axis* SpectraAxis::clone(const std::size_t length, const MatrixWorkspace* const parentWorkspace)
+std::size_t SpectraAxis::length() const
 {
-  UNUSED_ARG(parentWorkspace)
-    SpectraAxis * newAxis = new SpectraAxis(*this);
-  newAxis->m_values.clear();
-  newAxis->m_values.resize(length);
-  return newAxis;
+  return m_parentWS->getNumberHistograms();
 }
 
 /** Get the axis value at the position given
@@ -108,7 +69,7 @@ double SpectraAxis::operator()(const std::size_t& index, const std::size_t& vert
     throw Kernel::Exception::IndexError(index, length()-1, "SpectraAxis: Index out of range.");
   }
 
-  return static_cast<double>(m_values[index]);
+  return static_cast<double>(m_parentWS->getSpectrum(index)->getSpectrumNo());
 }
 
 /** Sets the axis value at a given position
@@ -123,23 +84,23 @@ void SpectraAxis::setValue(const std::size_t& index, const double& value)
     throw Kernel::Exception::IndexError(index, length()-1, "SpectraAxis: Index out of range.");
   }
 
-  m_values[index] = static_cast<specid_t>(value);
+  // TODO: Remove this evilness, preferably by removing the setValue method entirely
+  const_cast<MatrixWorkspace*>(m_parentWS)->getSpectrum(index)->setSpectrumNo(static_cast<specid_t>(value));
 }
 
 /** Returns the spectrum number at the position given (Spectra axis only)
  *  @param  index The position for which the value is required
  *  @return The spectrum number as an int
- *  @throw  domain_error If this method is called on a numeric axis
  *  @throw  IndexError If the index requested is not in the range of this axis
  */
-const specid_t& SpectraAxis::spectraNo(const std::size_t& index) const
+specid_t SpectraAxis::spectraNo(const std::size_t& index) const
 {
   if (index >= length())
   {
     throw Kernel::Exception::IndexError(index, length()-1, "SpectraAxis: Index out of range.");
   }
 
-  return m_values[index];
+  return m_parentWS->getSpectrum(index)->getSpectrumNo();
 }
 
 /** Returns a map where index is the key and spectra is the value
@@ -148,14 +109,14 @@ const specid_t& SpectraAxis::spectraNo(const std::size_t& index) const
  */
 void SpectraAxis::getIndexSpectraMap(index2spec_map& map)const
 {
-  size_t nel=m_values.size();
+  size_t nel = length();
 
   if (nel==0)
     throw std::runtime_error("getSpectraIndexMap(),  zero elements");
   map.clear();
   for (size_t i=0; i < nel; ++i )
   {
-    map.insert(std::make_pair(i, m_values[i]));
+    map.insert(std::make_pair(i, m_parentWS->getSpectrum(i)->getSpectrumNo()));
   }
 }
 
@@ -166,14 +127,14 @@ void SpectraAxis::getIndexSpectraMap(index2spec_map& map)const
  */
 void SpectraAxis::getSpectraIndexMap(spec2index_map& map)const
 {
-  size_t nel=m_values.size();
+  size_t nel = length();
 
   if (nel==0)
     throw std::runtime_error("getSpectraIndexMap(),  zero elements");
   map.clear();
   for (size_t i=0; i < nel; ++i )
   {
-    map.insert(std::make_pair(m_values[i],i));
+    map.insert(std::make_pair(m_parentWS->getSpectrum(i)->getSpectrumNo(),i));
   }
 }
 
@@ -192,7 +153,12 @@ bool SpectraAxis::operator==(const Axis& axis2) const
   {
     return false;
   }
-  return std::equal(m_values.begin(),m_values.end(),spec2->m_values.begin());
+  for ( size_t i = 0; i < length(); ++i )
+  {
+    if ( spectraNo(i) != axis2.spectraNo(i) ) return false;
+  }
+  // All good if we get to here
+  return true;
 }
 
 /** Returns a text label which shows the value at index and identifies the
@@ -205,5 +171,17 @@ std::string SpectraAxis::label(const std::size_t& index)const
   return "sp-" + boost::lexical_cast<std::string>(spectraNo(index));
 }
 
+/// returns min value defined on axis
+double SpectraAxis::getMin() const
+{
+  return m_parentWS->getSpectrum(0)->getSpectrumNo();
+}
+
+ /// returns max value defined on axis
+double SpectraAxis::getMax() const
+{
+  return m_parentWS->getSpectrum(length()-1)->getSpectrumNo();
+}
+
 } // namespace API
 } // namespace Mantid