Skip to content

Add documentation and test for eigensolver option.#5006

Merged
ye-luo merged 7 commits into
QMCPACK:developfrom
mdewing:eigensolver_docs
May 30, 2024
Merged

Add documentation and test for eigensolver option.#5006
ye-luo merged 7 commits into
QMCPACK:developfrom
mdewing:eigensolver_docs

Conversation

@mdewing

@mdewing mdewing commented May 24, 2024

Copy link
Copy Markdown
Contributor

Add a short test and documentation for the eigensolver option added in #4990 .

The existing tests cover the default inverse option. The added test is a copy of optm-batch-OneShiftOnly-general.xml, but using the general eigensolver.

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

Delete the items that do not apply

  • Testing changes (e.g. new unit/integration/performance tests)
  • Documentation or build script changes

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

desktop

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

  • Yes. This PR is up to date with current the current state of 'develop'
  • N/A. Code added or changed in the PR has been clang-formatted
  • Yes. This PR adds tests to cover any new code, or to catch a bug that is being fixed
  • Yes. Documentation has been added (if appropriate)

ye-luo
ye-luo previously approved these changes May 24, 2024
@ye-luo

ye-luo commented May 24, 2024

Copy link
Copy Markdown
Contributor

Test this please

@ye-luo ye-luo enabled auto-merge May 24, 2024 15:01

@prckent prckent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a deterministic or unit test for these?

It would be good to test the code path in CI and at least check we handle the option correctly.

@mdewing

mdewing commented May 24, 2024

Copy link
Copy Markdown
Contributor Author

Do you want a deterministic test to check that the right code path is being followed?
A unit test for that would be difficult, but a deterministic test that checks for the right strings in the output could work.

@prckent

prckent commented May 24, 2024

Copy link
Copy Markdown
Contributor

The second would work.

@ye-luo

ye-luo commented May 24, 2024

Copy link
Copy Markdown
Contributor

It is hard to maintain optimization tests in deterministic tests. I would request a unit test which setup a few matrices and then call the general and inverse routine and the result should be fully determinsitic. #4990 added what I was thinking of.

Check that the generalized eigensolver is chosen by checking for
matching strings in the output.
@prckent

prckent commented May 29, 2024

Copy link
Copy Markdown
Contributor

Test this please

@prckent

prckent commented May 29, 2024

Copy link
Copy Markdown
Contributor

Fatal Error. Aborting at QMCDriverFactory::createQMCDriver : method="linear_batch" is not safe with CPU mixed precision. Please use full precision build instead. Hmm. Wasn't aware of this one/had forgotten. CMakeLists just needs protecting.

Coverage uploads failed due to rate limits.

@mdewing

mdewing commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

The other option is to add "not safe with CPU mixed precision" to the passing regular expression list. Do you have a preference between this and guarding the CMake?

@mdewing

mdewing commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

Or, check the value of SUCCESS_STATUS_MP and set WILL_FAIL appropriately. That's probably the closest to the existing solution.

@prckent

prckent commented May 29, 2024

Copy link
Copy Markdown
Contributor

The latter one (SUCCESS STATUS MP, WILL FAIL) seems simplest and also checks that the code bails as expected.

@prckent

prckent commented May 29, 2024

Copy link
Copy Markdown
Contributor

Test this please

@prckent

prckent commented May 30, 2024

Copy link
Copy Markdown
Contributor

Test this please

@ye-luo ye-luo merged commit 5929a05 into QMCPACK:develop May 30, 2024
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.

3 participants