Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix assertion failure #4752

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Fix assertion failure #4752

merged 1 commit into from
Sep 29, 2023

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Sep 29, 2023

Proposed changes

Addresses #4666.
mw_evaluate_notranspose and evaluate_notranspose don't produce bitwise identical results.
The problem started by #4407 which introduce a batched evaluation and caused bitwise matrix value checking failure. I printed diff and found the value is at the epsilon of float or double. So no concern.

The intention of the assertion was to catch any missing D2H transfer earlier and there is no need of recomputing. So the solution is removing the re-computation.

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?

epyc-server

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'

Addresses QMCPACK#4666.
mw_evaluate_notranspose and evaluate_notranspose don't produce bitwise identical results.
The problem started by QMCPACK#4407 which introduce a batched evaluation and caused bitwise matrix value checking failure. I printed diff and found the value is at the epsilon of float or double. So no concern.

The intention of the assertion was to catch any missing D2H transfer earlier and there is no need of recomputing. So the solution is removing the re-computation.
@ye-luo
Copy link
Contributor Author

ye-luo commented Sep 29, 2023

Test this please

@prckent
Copy link
Contributor

prckent commented Sep 29, 2023

Thanks. An important source of worry removed. Did you find this by bisection or inspection?

@prckent
Copy link
Contributor

prckent commented Sep 29, 2023

Closes #4666

@ye-luo
Copy link
Contributor Author

ye-luo commented Sep 29, 2023

Thanks. An important source of worry removed. Did you find this by bisection or inspection?

After I printed the delta values and found nothing to worry, I had a rough idea about the cause and tested before/after merging #4407.

@prckent prckent merged commit dacfcfb into QMCPACK:develop Sep 29, 2023
37 checks passed
@ye-luo ye-luo deleted the fix-assertion branch September 29, 2023 15:27
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.

2 participants