Merge pull request #1897 from ghutchis/fixup-mo-window-bugs

avogadro/core/basisset.h

@@ -106,28 +106,15 @@ class AVOGADROCORE_EXPORT BasisSet
   virtual unsigned int molecularOrbitalCount(
     ElectronType type = Paired) const = 0;
 
-  /**
-   * Check if the given MO number is the HOMO or not.
-   * @param n The MO number.
-   * @return True if the given MO number is the HOMO.
-   */
-  bool homo(unsigned int n) { return n == homo(); }
-
   /**
    * @return The molecular orbital number corresponding to the HOMO orbital.
    */
-  unsigned int homo() const { return m_electrons[0] / 2; }
+  unsigned int homo(ElectronType type = Paired) const { return lumo(type) - 1; }
 
-  /**
-   * Check if the given MO number is the LUMO or not.
-   * @param n The MO number.
-   * @return True if the given MO number is the LUMO.
-   */
-  bool lumo(unsigned int n) { return n == lumo(); }
   /**
    * @return The molecular orbital number corresponding to the LUMO orbital.
    */
-  unsigned int lumo() const { return m_electrons[0] / 2 + 1; }
+  unsigned int lumo(ElectronType type = Paired) const;
 
   /**
    * @return True of the basis set is valid, false otherwise.
@@ -244,6 +231,30 @@ class AVOGADROCORE_EXPORT BasisSet
   std::vector<unsigned char> m_moOccupancy[2];
 };
 
+inline unsigned int BasisSet::lumo(ElectronType type) const
+{
+  if (type == Beta) {
+    // check if we have occupancy data
+    if (m_moOccupancy[1].size() > 0) {
+      for (unsigned int i = 0; i < m_moOccupancy[1].size(); ++i) {
+        if (m_moOccupancy[1][i] == 0)
+          return i;
+      }
+    }
+  } else {
+    // alpha or paired
+    // check if we have occupancy data - more accurate
+    if (m_moOccupancy[0].size() > 0) {
+      for (unsigned int i = 0; i < m_moOccupancy[0].size(); ++i) {
+        if (m_moOccupancy[0][i] == 0)
+          return i;
+      }
+    }
+  }
+  // fall back to electron count
+  return m_electrons[0] / 2 + 1;
+}
+
 inline void BasisSet::setElectronCount(unsigned int n, ElectronType type)
 {
   switch (type) {

avogadro/qtplugins/CMakeLists.txt

@@ -110,7 +110,7 @@ add_subdirectory(copypaste)
 add_subdirectory(cp2kinput)
 add_subdirectory(crystal)
 add_subdirectory(customelements)
-# add_subdirectory(dipole)
+add_subdirectory(dipole)
 add_subdirectory(editor)
 add_subdirectory(fetchpdb)
 add_subdirectory(focus)

avogadro/qtplugins/surfaces/orbitals.cpp

@@ -36,7 +36,21 @@ Orbitals::Orbitals(QObject* p)
   connect(m_action, SIGNAL(triggered()), SLOT(openDialog()));
 }
 
-Orbitals::~Orbitals() {}
+Orbitals::~Orbitals()
+{
+  // cancel the current running calculation
+  if (m_gaussianConcurrent) {
+    qDebug() << "Cancelling current calculation";
+    auto* watcher = &m_gaussianConcurrent->watcher();
+    if (watcher != nullptr && watcher->isRunning())
+      watcher->cancel();
+  }
+  delete m_runningMutex;
+
+  if (m_dialog)
+    m_dialog->deleteLater();
+  // molecule and basis are freed elsewhere
+}
 
 QList<QAction*> Orbitals::actions() const
 {
@@ -61,6 +75,10 @@ void Orbitals::setMolecule(QtGui::Molecule* mol)
   m_molecule = mol;
   // check if it has basis set data
   bool hasOrbitals = (m_molecule->basisSet() != nullptr);
+  // sanity check if there are actually mo coefficients
+  auto* basis = dynamic_cast<Core::GaussianSet*>(m_molecule->basisSet());
+  if (basis == nullptr || basis->moMatrix().size() == 0)
+    hasOrbitals = false;
 
   if (hasOrbitals)
     m_action->setEnabled(true);
@@ -76,6 +94,7 @@ void Orbitals::setMolecule(QtGui::Molecule* mol)
   // Stuff we manage that will not be valid any longer
   m_queue.clear();
   m_currentRunningCalculation = -1;
+  m_currentMeshCalculation = -1;
 
   if (m_basis) {
     delete m_basis;
@@ -84,7 +103,7 @@ void Orbitals::setMolecule(QtGui::Molecule* mol)
 
   loadBasis();
 
-  if (!m_basis || m_basis->electronCount() == 0)
+  if (!m_basis || m_basis->electronCount() == 0 || !hasOrbitals)
     return; // no electrons, no orbitals
 
   loadOrbitals();
@@ -103,6 +122,10 @@ void Orbitals::loadOrbitals()
   if (m_basis == nullptr || m_molecule == nullptr)
     return;
 
+  auto* basis = dynamic_cast<Core::GaussianSet*>(m_molecule->basisSet());
+  if (basis == nullptr || basis->moMatrix().size() == 0)
+    return;
+
   if (!m_dialog) {
     m_dialog = new OrbitalWidget(qobject_cast<QWidget*>(parent()), Qt::Window);
     connect(m_dialog, SIGNAL(orbitalSelected(unsigned int)), this,
@@ -125,6 +148,11 @@ void Orbitals::moleculeChanged(unsigned int changes)
   bool isEnabled = m_action->isEnabled();
   bool hasOrbitals = (m_molecule->basisSet() != nullptr);
 
+  // sanity check if there are actually mo coefficients
+  auto* basis = dynamic_cast<Core::GaussianSet*>(m_molecule->basisSet());
+  if (basis == nullptr || basis->moMatrix().size() == 0)
+    hasOrbitals = false;
+
   if (isEnabled != hasOrbitals) {
     m_action->setEnabled(hasOrbitals);
     if (hasOrbitals)
@@ -323,7 +351,9 @@ void Orbitals::calculateCube()
                << "\tOrbital " << cI->orbital << "\n"
                << "\tResolution " << cI->resolution;
 #endif
+      m_currentMeshCalculation = m_currentRunningCalculation;
       calculatePosMesh();
+      calculationComplete();
       return;
     }
   }
@@ -466,10 +496,15 @@ void Orbitals::renderOrbital(unsigned int row)
   // Find the most recent calc matching the selected orbital:
   calcInfo calc;
   int index = -1;
+  // in the event of ties, pick the best resolution
+  double resolution = OrbitalWidget::OrbitalQualityToDouble(0);
   for (int i = 0; i < m_queue.size(); i++) {
     calc = m_queue[i];
     if (calc.orbital == orbital && calc.state == Completed) {
-      index = i;
+      if (calc.resolution <= resolution) {
+        resolution = calc.resolution;
+        index = i;
+      }
     }
   }
 

avogadro/qtplugins/surfaces/orbitaltablemodel.cpp

@@ -61,18 +61,20 @@ QVariant OrbitalTableModel::data(const QModelIndex& index, int role) const
       return QString("%L1").arg(orb->energy, 0, 'f', 3);
     case C_Status: {
       // Check for divide by zero
+      int percent;
       if (orb->max == orb->min)
-        return 0;
-      int percent =
-        100 * (orb->current - orb->min) / float(orb->max - orb->min);
-      // Adjust for stages
-      int stages = (orb->totalStages == 0) ? 1 : orb->totalStages;
-      percent /= float(stages);
-      percent += (orb->stage - 1) * (100.0 / float(stages));
-      // clamp to 100%
-      if (percent > 100)
-        percent = 100;
-      return QString("%L1 %").arg(percent);
+        percent = 0;
+      else {
+        percent = 100 * (orb->current - orb->min) / float(orb->max - orb->min);
+        // Adjust for stages
+        int stages = (orb->totalStages == 0) ? 1 : orb->totalStages;
+        percent /= float(stages);
+        percent += (orb->stage - 1) * (100.0 / float(stages));
+        // clamp to 100%
+        if (percent > 100)
+          percent = 100;
+      }
+      return QString("%L1%").arg(percent);
     }
     case C_Symmetry:
       symbol = orb->symmetry;

avogadro/qtplugins/surfaces/orbitalwidget.cpp

@@ -44,6 +44,7 @@ OrbitalWidget::OrbitalWidget(QWidget* parent, Qt::WindowFlags f)
     SIGNAL(selectionChanged(const QItemSelection&, const QItemSelection&)),
     this, SLOT(tableClicked(const QItemSelection&)));
   connect(ui.push_render, SIGNAL(clicked()), this, SLOT(renderClicked()));
+
   // TODO: Implement configure dialog
   ui.push_configure->setVisible(false);
   connect(ui.push_configure, SIGNAL(clicked()), this, SLOT(configureClicked()));
@@ -171,7 +172,7 @@ void OrbitalWidget::renderClicked()
   QModelIndex first = selection.first();
   first = m_sortedTableModel->mapToSource(first);
 
-  int orbital = first.row() + 1;
+  int orbital = first.row(); // renderRequested handles the +1
   emit renderRequested(orbital, quality);
 }
 

tests/core/basissettest.cpp

@@ -17,17 +17,17 @@ TEST(BasisSetTest, homo)
 
   basis.setElectronCount(2, BasisSet::Paired);
   EXPECT_EQ(basis.homo(), 1);
-  EXPECT_TRUE(basis.homo(basis.homo()));
+  // EXPECT_TRUE(basis.homo(basis.homo()));
 
   EXPECT_EQ(basis.lumo(), 2);
-  EXPECT_TRUE(basis.lumo(basis.lumo()));
+  // EXPECT_TRUE(basis.lumo(basis.lumo()));
 
   basis = SlaterSet();
   basis.setElectronCount(2, BasisSet::Alpha);
   basis.setElectronCount(1, BasisSet::Beta);
 
   EXPECT_EQ(basis.homo(), 1);
-  EXPECT_TRUE(basis.homo(basis.homo()));
+  //  EXPECT_TRUE(basis.homo(basis.homo()));
 
   // This is broken: the lumo could be either the
   // next alpha or the next beta depending on the