Merge pull request #5414 from rcclay/remove_history_method

prckent · web-flow · commit 1b3a0eabf9ba · 2025-04-03T13:35:07.000-04:00
Remove history method from RotatedSPOs
diff --git a/docs/intro_wavefunction.rst b/docs/intro_wavefunction.rst
@@ -951,8 +951,6 @@ Attribute:
 +=================+==========+================+=========+====================================+
 | ``name``        | Text     |                |         | Name of rotated SPOSet             |
 +-----------------+----------+----------------+---------+------------------------------------+
-| ``method``      | Text     | global/history | global  | Rotation matrix composition method |
-+-----------------+----------+----------------+---------+------------------------------------+
 
 .. code-block::
    :caption: Orbital Rotation XML element.
@@ -989,14 +987,7 @@ These parameters are a subset of the full number of parameters in the kappa matr
 When rotations are combined, the entries corresponding to zero parameter derivatives can
 take on a non-zero value (i.e. the kappa matrix gets 'filled-in').
 
-There are two ways to handle this.
-One way is to store a list of applied rotations.
-This method applies a new rotation to the coefficient matrix, and updates the coefficient matrix at each optimization step.
-This is the "history" method.
-
-.. math:: C' = \exp(\kappa_n) \dots \exp(\kappa_1) \exp(\kappa_0) C
-
-The other way is to track the full set of kappa values separately.
+QMCPACK handles this problem by tracking the full set of kappa values separately.
 After the matrix multiplication to compose the rotations, the matrix log recovers the new kappa matrix entries.
 This is the "global" method.
 This method keeps a separate copy of the coefficient matrix and updates it using the global rotation matrix at each optimization step.
diff --git a/src/QMCWaveFunctions/RotatedSPOs.cpp b/src/QMCWaveFunctions/RotatedSPOs.cpp
@@ -125,50 +125,22 @@ void RotatedSPOs::resetParametersExclusive(const opt_variables_type& active)
     myVars[i]                 = active[loc];
   }
 
-  if (use_global_rot_)
-  {
-    std::vector<ValueType> old_param(m_full_rot_inds_.size());
-    std::copy_n(myVarsFull_.data(), myVarsFull_.size(), old_param.data());
-
-    applyDeltaRotation(delta_param, old_param, myVarsFull_);
-  }
-  else
-  {
-    apply_rotation(delta_param, false);
+  std::vector<ValueType> old_param(m_full_rot_inds_.size());
+  std::copy_n(myVarsFull_.data(), myVarsFull_.size(), old_param.data());
 
-    // Save the parameters in the history list
-    history_params_.push_back(delta_param);
-  }
+  applyDeltaRotation(delta_param, old_param, myVarsFull_);
 }
 
 void RotatedSPOs::writeVariationalParameters(hdf_archive& hout)
 {
   hout.push("RotatedSPOs");
-  if (use_global_rot_)
-  {
-    hout.push("rotation_global");
-    const std::string rot_global_name = std::string("rotation_global_") + SPOSet::getName();
 
-    hout.write(myVarsFull_, rot_global_name);
-    hout.pop();
-  }
-  else
-  {
-    hout.push("rotation_history");
-    size_t rows = history_params_.size();
-    size_t cols = 0;
-    if (rows > 0)
-      cols = history_params_[0].size();
-
-    Matrix<ValueType> tmp(rows, cols);
-    for (size_t i = 0; i < rows; i++)
-      for (size_t j = 0; j < cols; j++)
-        tmp[i][j] = history_params_[i][j];
-    std::string rot_hist_name = std::string("rotation_history_") + SPOSet::getName();
-    hout.write(tmp, rot_hist_name);
-    hout.pop();
-  }
+  hout.push("rotation_global");
+  const std::string rot_global_name = std::string("rotation_global_") + SPOSet::getName();
 
+  hout.write(myVarsFull_, rot_global_name);
+  hout.pop();
+  
   // Save myVars in order to restore object state exactly
   //  The values aren't meaningful, but they need to match those saved in VariableSet
   hout.push("rotation_params");
@@ -189,11 +161,7 @@ void RotatedSPOs::readVariationalParameters(hdf_archive& hin)
 {
   hin.push("RotatedSPOs", false);
 
-  bool grp_hist_exists   = hin.is_group("rotation_history");
   bool grp_global_exists = hin.is_group("rotation_global");
-  if (!grp_hist_exists && !grp_global_exists)
-    app_warning() << "Rotation parameters not found in VP file";
-
 
   if (grp_global_exists)
   {
@@ -217,29 +185,9 @@ void RotatedSPOs::readVariationalParameters(hdf_archive& hin)
 
     applyFullRotation(myVarsFull_, true);
   }
-  else if (grp_hist_exists)
+  else 
   {
-    hin.push("rotation_history", false);
-    std::string rot_hist_name = std::string("rotation_history_") + SPOSet::getName();
-    std::vector<int> sizes(2);
-    if (!hin.getShape<ValueType>(rot_hist_name, sizes))
-      throw std::runtime_error("Failed to read rotation history in VP file");
-
-    int rows = sizes[0];
-    int cols = sizes[1];
-    history_params_.resize(rows);
-    Matrix<ValueType> tmp(rows, cols);
-    hin.read(tmp, rot_hist_name);
-    for (size_t i = 0; i < rows; i++)
-    {
-      history_params_[i].resize(cols);
-      for (size_t j = 0; j < cols; j++)
-        history_params_[i][j] = tmp(i, j);
-    }
-
-    hin.pop();
-
-    applyRotationHistory();
+    throw std::runtime_error("Error.  No global rotation group in h5.  Abort.");
   }
 
   hin.push("rotation_params", false);
@@ -290,8 +238,8 @@ void RotatedSPOs::buildOptVariables(const size_t nel)
     RotationIndices created_m_act_rot_inds;
 
     RotationIndices created_full_rot_inds;
-    if (use_global_rot_)
-      createRotationIndicesFull(nel, nmo, created_full_rot_inds);
+
+    createRotationIndicesFull(nel, nmo, created_full_rot_inds);
 
     createRotationIndices(nel, nmo, created_m_act_rot_inds);
 
@@ -306,13 +254,9 @@ void RotatedSPOs::buildOptVariables(const RotationIndices& rotations, const Rota
   // create active rotations
   m_act_rot_inds_ = rotations;
 
-  if (use_global_rot_)
-    m_full_rot_inds_ = full_rotations;
+  m_full_rot_inds_ = full_rotations;
 
-  if (use_global_rot_)
-    app_log() << "Orbital rotation using global rotation" << std::endl;
-  else
-    app_log() << "Orbital rotation using history" << std::endl;
+  app_log() << "Orbital rotation using global rotation" << std::endl;
 
   // This will add the orbital rotation parameters to myVars
   // and will also read in initial parameter values supplied in input file
@@ -349,13 +293,10 @@ void RotatedSPOs::buildOptVariables(const RotationIndices& rotations, const Rota
       registerParameter(i, p, q, myVars, params_, false);
   }
 
-  if (use_global_rot_)
-  {
-    const size_t nfull_rot = m_full_rot_inds_.size();
-    myVarsFull_.resize(nfull_rot);
-    for (int i = 0; i < nfull_rot; i++)
-      myVarsFull_[i] = (params_supplied_ && i < m_act_rot_inds_.size()) ? params_[i] : 0.0;
-  }
+  const size_t nfull_rot = m_full_rot_inds_.size();
+  myVarsFull_.resize(nfull_rot);
+  for (int i = 0; i < nfull_rot; i++)
+    myVarsFull_[i] = (params_supplied_ && i < m_act_rot_inds_.size()) ? params_[i] : 0.0;
 
   //Printing the parameters
   if (true)
@@ -464,14 +405,6 @@ void RotatedSPOs::applyFullRotation(const std::vector<ValueType>& full_param, bo
   Phi_->applyRotation(rot_mat, use_stored_copy);
 }
 
-void RotatedSPOs::applyRotationHistory()
-{
-  for (auto delta_param : history_params_)
-  {
-    apply_rotation(delta_param, false);
-  }
-}
-
 // compute exponential of a real, antisymmetric matrix by diagonalizing and exponentiating eigenvalues
 void RotatedSPOs::exponentiate_antisym_matrix(ValueMatrix& mat)
 {
@@ -1640,8 +1573,6 @@ std::unique_ptr<SPOSet> RotatedSPOs::makeClone() const
   myclone->m_full_rot_inds_ = this->m_full_rot_inds_;
   myclone->myVars           = this->myVars;
   myclone->myVarsFull_      = this->myVarsFull_;
-  myclone->history_params_  = this->history_params_;
-  myclone->use_global_rot_  = this->use_global_rot_;
   return myclone;
 }
 
diff --git a/src/QMCWaveFunctions/RotatedSPOs.h b/src/QMCWaveFunctions/RotatedSPOs.h
@@ -23,7 +23,6 @@ namespace testing
 {
 const opt_variables_type& getMyVars(RotatedSPOs& rot);
 const std::vector<QMCTraits::ValueType>& getMyVarsFull(RotatedSPOs& rot);
-const std::vector<std::vector<QMCTraits::ValueType>>& getHistoryParams(RotatedSPOs& rot);
 } // namespace testing
 
 class RotatedSPOs : public SPOSet, public OptimizableObject
@@ -99,10 +98,6 @@ class RotatedSPOs : public SPOSet, public OptimizableObject
                                      std::vector<ValueType>& new_param,
                                      ValueMatrix& new_rot_mat);
 
-  // When initializing the rotation from VP files
-  // This function applies the rotation history
-  void applyRotationHistory();
-
   // This function applies the global rotation (similar to apply_rotation, but for the full
   // set of rotation parameters)
   void applyFullRotation(const std::vector<ValueType>& full_param, bool use_stored_copy);
@@ -400,9 +395,6 @@ class RotatedSPOs : public SPOSet, public OptimizableObject
   //  void evaluateThirdDeriv(const ParticleSet& P, int first, int last, GGGMatrix& grad_grad_grad_logdet)
   //  {Phi->evaluateThridDeriv(P, first, last, grad_grad_grad_logdet); }
 
-  /// Use history list (false) or global rotation (true)
-  void set_use_global_rotation(bool use_global_rotation) { use_global_rot_ = use_global_rotation; }
-
   void mw_evaluateDetRatios(const RefVectorWithLeader<SPOSet>& spo_list,
                             const RefVectorWithLeader<const VirtualParticleSet>& vp_list,
                             const RefVector<ValueVector>& psi_list,
@@ -472,17 +464,10 @@ class RotatedSPOs : public SPOSet, public OptimizableObject
   /// timer for apply_rotation
   NewTimer& apply_rotation_timer_;
 
-  /// List of previously applied parameters
-  std::vector<std::vector<ValueType>> history_params_;
-
   static RefVectorWithLeader<SPOSet> extractPhiRefList(const RefVectorWithLeader<SPOSet>& spo_list);
 
-  /// Use global rotation or history list
-  bool use_global_rot_ = true;
-
   friend const opt_variables_type& testing::getMyVars(RotatedSPOs& rot);
   friend const std::vector<ValueType>& testing::getMyVarsFull(RotatedSPOs& rot);
-  friend const std::vector<std::vector<ValueType>>& testing::getHistoryParams(RotatedSPOs& rot);
 };
 
 
diff --git a/src/QMCWaveFunctions/SPOSetBuilder.cpp b/src/QMCWaveFunctions/SPOSetBuilder.cpp
@@ -130,7 +130,6 @@ std::unique_ptr<SPOSet> SPOSetBuilder::createRotatedSPOSet(xmlNodePtr cur)
   std::string method;
   OhmmsAttributeSet attrib;
   attrib.add(spo_object_name, "name");
-  attrib.add(method, "method", {"global", "history"});
   attrib.put(cur);
 
   std::unique_ptr<SPOSet> sposet;
@@ -151,9 +150,6 @@ std::unique_ptr<SPOSet> SPOSetBuilder::createRotatedSPOSet(xmlNodePtr cur)
   sposet->storeParamsBeforeRotation();
   auto rot_spo = std::make_unique<RotatedSPOs>(spo_object_name, std::move(sposet));
 
-  if (method == "history")
-    rot_spo->set_use_global_rotation(false);
-
   processChildren(cur, [&](const std::string& cname, const xmlNodePtr element) {
     if (cname == "opt_vars")
     {
diff --git a/src/QMCWaveFunctions/tests/test_RotatedSPOs.cpp b/src/QMCWaveFunctions/tests/test_RotatedSPOs.cpp
@@ -732,7 +732,6 @@ namespace testing
 {
 const opt_variables_type& getMyVars(RotatedSPOs& rot) { return rot.myVars; }
 const std::vector<QMCTraits::ValueType>& getMyVarsFull(RotatedSPOs& rot) { return rot.myVarsFull_; }
-const std::vector<std::vector<QMCTraits::ValueType>>& getHistoryParams(RotatedSPOs& rot) { return rot.history_params_; }
 } // namespace testing
 
 // Test using global rotation
@@ -787,55 +786,6 @@ TEST_CASE("RotatedSPOs read and write parameters", "[wavefunction]")
     CHECK(full_var[i] == ValueApprox(vs_values[i]));
 }
 
-// Test using history list.
-TEST_CASE("RotatedSPOs read and write parameters history", "[wavefunction]")
-{
-  //Problem with h5 parameter parsing for complex build.  To be fixed in future PR.
-  auto fake_spo = std::make_unique<FakeSPO<QMCTraits::ValueType>>();
-  fake_spo->setOrbitalSetSize(4);
-  RotatedSPOs rot("fake_rot", std::move(fake_spo));
-  rot.set_use_global_rotation(false);
-  int nel = 2;
-  rot.buildOptVariables(nel);
-
-  std::vector<SPOSet::ValueType> vs_values{0.1, 0.15, 0.2, 0.25};
-
-  optimize::VariableSet vs;
-  rot.checkInVariablesExclusive(vs);
-  auto* vs_values_data_real = (SPOSet::RealType*)vs_values.data();
-  for (size_t i = 0; i < vs.size(); i++)
-    vs[i] = vs_values_data_real[i];
-  rot.resetParametersExclusive(vs);
-
-  {
-    hdf_archive hout;
-    vs.writeToHDF("rot_vp_hist.h5", hout);
-
-    rot.writeVariationalParameters(hout);
-  }
-
-  auto fake_spo2 = std::make_unique<FakeSPO<QMCTraits::ValueType>>();
-  fake_spo2->setOrbitalSetSize(4);
-
-  RotatedSPOs rot2("fake_rot", std::move(fake_spo2));
-  rot2.buildOptVariables(nel);
-
-  optimize::VariableSet vs2;
-  rot2.checkInVariablesExclusive(vs2);
-
-  hdf_archive hin;
-  vs2.readFromHDF("rot_vp_hist.h5", hin);
-  rot2.readVariationalParameters(hin);
-
-  auto& var = testing::getMyVars(rot2);
-  for (size_t i = 0; i < var.size(); i++)
-    CHECK(var[i] == Approx(vs[i]));
-
-  const auto hist = testing::getHistoryParams(rot2);
-  REQUIRE(hist.size() == 1);
-  REQUIRE(hist[0].size() == 4);
-}
-
 template<typename T>
 class DummySPOSetWithoutMW : public SPOSetT<T>
 {
diff --git a/src/QMCWaveFunctions/tests/test_RotatedSPOs_LCAO.cpp b/src/QMCWaveFunctions/tests/test_RotatedSPOs_LCAO.cpp
@@ -381,15 +381,15 @@ TEST_CASE("Rotated LCAO WF2 with jastrow", "[qmcapp]")
         </atomicBasisSet>
       </basisset>
       <rotated_sposet name="rot-spo-up">
-        <sposet basisset="LCAOBSet" name="spo-up" method="history">
+        <sposet basisset="LCAOBSet" name="spo-up">
           <coefficient id="updetC" type="Array" size="2">
             1.0 0.0
             0.0 1.0
           </coefficient>
         </sposet>
       </rotated_sposet>
       <rotated_sposet name="rot-spo-down">
-        <sposet basisset="LCAOBSet" name="spo-down" method="history">
+        <sposet basisset="LCAOBSet" name="spo-down">
           <coefficient id="updetC" type="Array" size="2">
             1.0 0.0
             0.0 1.0
