colvars icon indicating copy to clipboard operation
colvars copied to clipboard

Deprecate "enableFitGradients no"

Open HanatoK opened this issue 2 months ago • 7 comments

This PR aims to complete https://github.com/Colvars/colvars/issues/850 by adding the tests at first.

However, I was not quite sure how to add new tests with only enableFitGradients changed to yes, so I copied the files into input_files manually. Please let me know if you have a better idea to change the test files while keeping the old enableFitGradients no configuration for the time being.

Update: For the time being, in commit https://github.com/Colvars/colvars/pull/869/commits/26918057552c95cc823e1223fe43f10a18feb9c2, I have added a warning for enableFitGradients no.

Tasks:

  • [ ] Remove the tests with enableFitGradients no from the stub tests;
  • [ ] Update the NAMD tests;
  • [ ] Update the GROMACS tests;
  • [ ] Update the LAMMPS tests.

HanatoK avatar Oct 17 '25 16:10 HanatoK

Looking at this, I wonder if we could just switch this flag on for all functional tests. That would be simpler.

jhenin avatar Oct 22 '25 14:10 jhenin

Looking at this, I wonder if we could just switch this flag on for all functional tests. That would be simpler.

Honestly I don't know what to do with enableFitGradients. To phase out enableFitGradients no, we need to update all reference data for all MD backends, and update the documentation to tell the users that enableFitGradients no is ignored, but perhaps some users would still argue that in their calculations the fit gradients could be canceled out and they want to skip them.

HanatoK avatar Oct 22 '25 15:10 HanatoK

Yes, I'm in favor of removing it from the tests and regenerating all reference data that requires it.

We don't need to remove the keyword entirely. We can deprecate it by removing its entry from the documentation and adding a release note, e.g. on a dedicated Wiki page. So we won't break anyone's workflow, but we will simplify the work of new users who won't have to think about it.

jhenin avatar Oct 23 '25 11:10 jhenin

I'm still favorable to deprecating the keyword and testing only the fit gradient case.

jhenin avatar Nov 18 '25 17:11 jhenin

Hi @jhenin and @giacomofiorin ! Should I regenerate the test data without enableFitGradients no for all backends in this PR, or in a separate one?

HanatoK avatar Dec 02 '25 15:12 HanatoK

I would use this PR, for simplicity. You can change the title.

jhenin avatar Dec 02 '25 16:12 jhenin

I would use this PR, for simplicity. You can change the title.

OK. I have changed the title and added a list of TODOs.

HanatoK avatar Dec 02 '25 17:12 HanatoK