molovol / MoloVol

MoloVol is a free, cross-plattform, scientific software for volume and surface computations of single molecules and crystallographic unit cells.
https://molovol.com
MIT License
22 stars 4 forks source link

Default value for two probe mode doesn't work properly #64

Closed jmaglic closed 3 years ago

jmaglic commented 3 years ago

Description

When I enable "two probe" mode in the UI, there is already a preset value in the "probe 2 radius" field. However, when I start the calculation this value is not used. I have to actively change the default value, so that it works properly.

Apart from this there is another issue: when I disable the two probe mode, the input field for probe 2 radius is greyed out and cannot be edited. However, when I start the calculation I get an error that probe 1 has to be smaller than probe 2. In order to fix this I have to enable the two probe mode option, change the value of the probe 2 radius, and then uncheck the option again. Worst of all, the program freezes if I don't uncheck the option before starting the calculation.

Proposed change

UI Overhaul.

rlavendomme commented 3 years ago

When I enable "two probe" mode in the UI, there is already a preset value in the "probe 2 radius" field. However, when I start the calculation this value is not used. I have to actively change the default value, so that it works properly.

Strange, as long as the two probe mode is activated, the value should be taken. I had no issue with that on the alt-voxel-type-find branch when compiled on windows.

The code in base_guicontrol.cpp is pretty simple

double MainFrame::getProbe2Radius(){
  if(getProbeMode()){
    return std::stod(probe2InputText->GetValue().ToStdString());
  }
  else{ // when the probe mode is set to one probe, the second probe radius is considered null
    return 0;
  }
}

Apart from this there is another issue: when I disable the two probe mode, the input field for probe 2 radius is greyed out and cannot be edited. However, when I start the calculation I get an error that probe 1 has to be smaller than probe 2. In order to fix this I have to enable the two probe mode option, change the value of the probe 2 radius, and then uncheck the option again. Worst of all, the program freezes if I don't uncheck the option before starting the calculation.

Also strange since these error messages should only come when two probe mode is checked (even when r_probe2 < r_probe1). From model.cpp

bool Model::setProbeRadii(const double& r_1, const double& r_2, bool two_probe_mode){
  _r_probe1 = r_1;
  if (two_probe_mode){
    if (r_1 > r_2){
    Ctrl::getInstance()->notifyUser("Probes radii invalid!\nSet probe 2 radius > probe 1 radius.");
    return false;
    }
    else{
      _r_probe2 = r_2;
    }
  }
  return true;
}

It seems like in your tests, the two probe mode checkbox value didn't update properly. Can these issues be reproduced easily ?

jmaglic commented 3 years ago

I have found the solution. I'm not sure the issue was in the last stable build, I suspect I may have introduced in one of my earlier commits on my most recent development branch. The issue was that the probe radii were updated before the checkbox was evaluated, so the app was still behaving according to the state of the checkbox from the previous calculation.