In Code/Numerics/Optimizer/BFGSOpt.h, the gradient-convergence check
computed
double term = std::max(funcVal * gradScale, 1.0);
...
test /= term;
if (test < gradTol) return 0;
When funcVal (the current energy) is negative, funcVal * gradScale is
negative and std::max clamps the denominator to 1.0. The convergence
test therefore divides the gradient norm by 1 instead of by the
intended |E| * gradScale, which over-tightens the criterion by a factor
of |funcVal * gradScale| whenever |funcVal * gradScale| > 1.
Negative energies are a normal mid-minimization state for force fields
that include stabilizing terms (MMFF94, UFF with charges, AMBER-style
potentials), so this affects realistic workloads: extra BFGS iterations
or, occasionally, hitting MAXITS and returning the "too many iterations"
status when convergence would otherwise have been reached.
The fix is to use |funcVal| in the denominator, matching the pattern
used three lines below ('std::max(fabs(pos[i]), 1.0)') and matching the
intended interpretation as a magnitude.
A new test case 'testBFGSOptimizationNegativeEnergy' in
testOptimizer.cpp minimizes a 2D quadratic whose value is always
negative along the convergence path and verifies the optimizer reaches
the analytic minimum.
git blame attributes the original line to commit e08e0d16d (Nov 2015),
when the optimizer was restructured; the surrounding code does use
absolute values, so this reads as an oversight rather than an
intentional choice.
* some optimizations of AlignPoints()
still should do some benchmarking to see how much of a difference the changes make
* remove unused variable
---------
Co-authored-by: = <=>
* constexpr Point, Point2D, and Point3D
* constexpr Vector
* cleanup
* rollback overzealous constexpring
* dial back the constexpr to get windows builds working
the math stuff isn't constexpr with MSVC++
* add [[nodiscard]]
* throw if close to zero
* fix moldraw2DTestCatch
* Fix testRGroupDecomp
* fix one test in distGeomHelpersCatch
* fix tests in distGeomHelpersCatch
* retry finding a dir vector when adding Hs
* push UFF fixes to calculateCosY
* fix the setTerminalAtomCoords deg 4 patch
* add a test
* reduce zero tolerance
* fixes
* do not leak MolCatalogParams
* do not leak points on align failures
* give python ownership of pointers returned in getFingerprintsHelper
* clean up ScaffoldNetwork ptr if createNetworkHelper fails
* manage FF ptrs during construction
* wire in ownsBondInvGenerator in getMorganGenerator
* manage weights in rdMolAlign CalcRMS
* fix ownership of matches list/tuple in generateRmsdTransMatchPyTuple
* manage stream in createForwardSupplier during construction
* drop redundant Point3D allocations in GetUSRDistributionsFromPoints
* fix signed comparison mismatch
* remove some more deprecated numpy stuff
* workaround for changes to random.shuffle in python 3.11
* fix pickling of rdkit mols in python 3.11
* add py311 build to CI
* update py311 CI
* remove qt for py311 for the moment
* only use the new code with pyversion >=3.11
* use the new logic for all pickle_suites
* need to work with older py too
* very basics: actually parsing the new atom stereochem features
* add some input verification for the chiral permutations
* fix a typo
add quadruple bond SMILES/SMARTS extension
* add forgotten files
* patch from Roger
* add Roger's parsing examples
* typo
* new tests
* adjusted version of next PR from Roger:
- add SP2D hybridization for square planar (this may change)
- some modernizationof Chirality.cpp
- stop using < HybridizationType in Chirality.cpp (should probably do this elsewhere too)
- improved handling of hybridization assignment for new stereochem
- handle new stereo/hybridization in UFF
- tests for the above
* perception of non-tetrahedral stereo from 3D (from Roger S)
Basic testing of SP and TB based on opensmiles docs
* potential fixes for octahedral assignment
more tests
* docs update
need way more!
* map the TH tags directly to @ tags
* very basics of SMILES writing
this does not work with anything that changes the permutation order
like canonicalization or writing things in rings.
* start to support the getChiralAcross API
* more testing
* consistency
* add hasNonTetrahedralStereo() and getIdealAngleBetweenLigands()
* assignStereochemistry should only remove non-tetrahedral stereo
* re-simplify those tests
* cleanup matrix stream output
* initial pass at supporting nontet stereo in distgeom
* backup
* start on the reference docs
* TBP reference
* first pass at Oh finished
* update SP section
* more doc updates
* fix a typo
* add param to not remove Hs connected to non-tetrahedral atoms
* VERY basic coord generation for square planar
* TBP basics
* basic OH depiction
* start testing missing ligands
allow non-tet stereo in rings (ugly, but correct)
* add new TBP functions from Roger
* update depiction code for new API
* backup, the new tests work so far
* Finish the TB tests
* OH tests pass too
* cleanup
* first pass at getting correct SMILES with reordering
need way more testing than this
* ensure permutation 0 is correctly preserved
* some progress towards adding non-tetrahedral stereo to StereoInfo
* doc update
* add non-tet chiral classes to python wrappers
* make sure removeAllHs also gets neighbors of non-tetrahedral centers
more testing
* a bit of depictor cleanup
* make the assignment from 3D more tolerant
more testing
* improve the bulk testing
* cleanup
* remove a bit of redundant code
* ensure we don't write bogus permutation values to SMILES
* fix some rebase problems
* allow assignStereochemistryFrom3D() to be called without sanitization
* allow disabling the non-tetrahedral stereo when it's not explicit
* get that working on windows too
* collection of doxygen fixes
* more files with fixed doxygen comments
* more doxygen fixes
* more doxygen fixes
* last set of doxygen comment fixes
Co-authored-by: Jason Biggs <jasonb@wolfram.com>
* Remove accidentally tracked files and unset x flag
* Ignore ComicNeue
* Unify test tag to `reader`
* Trivial destructors
* Bump CMAKE_CXX_STANDARD to 14 (#4165)
* run clang-tidy with modernize-use-default-member-init
* results from modernize-use-emplace
* one uniform initialization per line
otherwise SWIG is unhappy
Co-authored-by: Brian Kelley <fustigator@gmail.com>
* change minimal cmake version to a consistent 3.5
* progress towards a cleanup
* get the basic python deps working
* two more libs
* another round of changes
all tests pass at this point
* next round of changes
all tests pass at this point
* close to done
all tests pass
* very close
* almost done
* shift the RDBoost dependencies around a bit
* remove an extraneous python linkage
this is trying to get the mac builds working again
* Only link to python if it was built shared (#3091)
* change in response to review
Co-Authored-By: Ric <ricrogz@users.noreply.github.com>
* move that suppression of the maybe-uninitialized warning to BoostStartInclude.h
Co-authored-by: Brian Kelley <fustigator@gmail.com>
Co-authored-by: Ric <ricrogz@users.noreply.github.com>
* run clang-tidy with readability-braces-around-statements
clang-format the results
clean up all the parts that clang-tidy-8 broke
* fix problem on windows
* disable builds of the StructChecker code by default
* operator"" _smarts() doesn't need to catch sanitization errors
* remove unused function
* turn back on some tests that shouldn't have been disabled
* Remove unused code from SMARTS parser and simplify a bit
SmilesParseOps::AddFragToMol is now used only from the SMARTS parser, so we can simplify the API
* Removes obsolete special case code for SMARTS
This was relevant when organic atoms in SMARTS queries were stored as two-part queries.
* improve SMARTS testing
make sure we can generate SMARTS from all the examples and then parse that again.
* Fixes#2814
* Fixes#2815
* some additional smarts tests to improve coverage
* test copy ctor and getPos
* remove obsolete test_list files
* include tests for the morgan invariant generators
* more cleanups and coverage improvements
* remove files that were mistakenly added
* basic conrec integration
* basic conrec integration
* add molecule contouring
* update
* add an alpha channel to the color
* allow filling the grid
* cleanup
* remove some debugging output
* first pass at getting similarity maps to actually work
* updates
* add another test
explicitly set linewidth to 1
* some SVG cleanup
* add option for extra padding
* update expected test results
* efficiency
* at least something of a test
* forgot an import
* fix SWIG wrappers
* cleanups in response to review
* allow calling the functions without providing levels
* allow contouring numpy grids
* remove all of the "from __future__" imports
* remove the first batch of rdkit.six imports/uses
* next step of rdkit.six removal
* removing xrange, range, and some maps
* next round of removals
* next round of cleanups
* fix inchi test
* last bits of "from rdkit.six" are gone
* and the last of the six stuff is gone
* strange importlib problem
* Fixes atom documentation
* Fixes#1461
This is a complicated one. Basically URANGE_CHECK when
used on unsigned integers has a problem when the size of
the range it’s checking is 0. The standard operations is
to check
URANGE(num, size-1)
Which (for unsigned integers) obviously rolls over.
This fixes all usage cases to be
URANGE(num+1, size)
And fixes the bugs found. (addBond and the mmff tests)
* Fixes#1461 - Updates URANGE_CHECK to be 0<=x<hi
