Skip to content

DescentEngine pointer and initialization fixes#6044

Closed
prckent wants to merge 1 commit into
QMCPACK:developfrom
prckent:fix_descent
Closed

DescentEngine pointer and initialization fixes#6044
prckent wants to merge 1 commit into
QMCPACK:developfrom
prckent:fix_descent

Conversation

@prckent

@prckent prckent commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Closes #6040

Fix two minor bugs that were tripping debug builds. Our non-debug sanitizer builds did not catch them

  1. Ensure _le_list, _vg, and _weight are created/initialized before first use. Set to length (1) in initializer list. They are subsequently resized to the correct thread count.

  2. Avoid deferencing of a null descentEngineObj by changing the API. It was deferenced in e.g. the adaptive optimizer even when no descentEngineObj would be available or needed. Updating the API seemed the cleanest way to handle this.

All the short-H4 tests were verified as working under debug builds after this change.

What type(s) of changes does this code introduce?

  • Bugfix

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

macos, macports gcc 13.2.

Checklist

    • I have read the pull request guidance and develop docs
    • This PR is up to date with the current state of 'develop'
    • Code added or changed in the PR has been clang-formatted
    • This PR adds tests to cover any new code, or to catch a bug that is being fixed
    • Documentation has been added (if appropriate)

@prckent

prckent commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Noting initial CI failures for Clang16-NoMPI-Offload-Complex


The following tests FAILED:
Errors while running CTest
	 31 - deterministic-unit_test_wavefunction_jastrow (Failed) deterministic quality_unknown unit
	 32 - deterministic-unit_test_wavefunction_determinant (Failed) deterministic quality_unknown unit

These are unrelated to the changed made here.

Earlier we had a CI failure for GCC12-NoMPI-Werror-Complex-Mixed in #6042 with similar symptoms that went away on rerun:

 81/890 Test   #32: deterministic-unit_test_wavefunction_determinant .....................................................***Failed    1.17 sec
QMCPACK printout is suppressed. Use --turn-on-printout to see all the printout.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_wavefunction_determinant is a Catch v2.13.10 host application.
Run with -? for options

-------------------------------------------------------------------------------
SmallMatrixDetCalculator::evaluate-Small
-------------------------------------------------------------------------------
/__w/qmcpack/qmcpack/src/QMCWaveFunctions/tests/test_multi_dirac_determinant.cpp:78
...............................................................................

/__w/qmcpack/qmcpack/src/QMCWaveFunctions/tests/test_multi_dirac_determinant.cpp:94: FAILED:
  CHECK( double_test.generic_evaluate(1<<7) == Approx(-1.3432116824) )
with expansion:
  -2231745156307.724609375
  ==
  Approx( -1.3432116824 )

/__w/qmcpack/qmcpack/src/QMCWaveFunctions/tests/test_multi_dirac_determinant.cpp:95: FAILED:
  CHECK( double_test.generic_evaluate(1<<12) == Approx(-1.3586431786) )
with expansion:
  inf == Approx( -1.3586431786 )

=> it is likely than a recent PR removed the initialization of a variable, or similar.

@ye-luo

ye-luo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Based on the added error trap, nullptr is never intended to be a legit case. Then it should not be allowed in the first place and we should fix the bug if there is one.

In general, changing reference to pointer is a step backward and harmful unless nullptr is a legit use case and then we need to protect every use of the pointer. So point 2 changes won't make our code safer.

@ye-luo

ye-luo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Point 1 seem to be an fix to #5913

@prckent

prckent commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Super. How about I make a fresh PR with just the constructor fix and we can decide on a proper solution to the other problem?

It does appear that we have some unreliability introduced into the CI by a recent PR => it is worth understanding the origins of that first since it is more general and is now breaking CI on develop.

@ye-luo

ye-luo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Super. How about I make a fresh PR with just the constructor fix and we can decide on a proper solution to the other problem?

please do so. thanks

@prckent prckent closed this Jul 15, 2026
@prckent
prckent deleted the fix_descent branch July 15, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some optimizers crash on startup in debug builds

2 participants