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

Build ppconvert executable when configured #2904

Merged
merged 4 commits into from
Feb 12, 2021
Merged

Conversation

prckent
Copy link
Contributor

@prckent prckent commented Feb 11, 2021

Proposed changes

When BUILD_PPCONVERT is enabled, build the executable by default. In case of problems, user can simply set BUILD_PPCONVERT=0. Enables ppconvert test to easily run.

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

  • Build related changes
  • Testing changes (e.g. new unit/integration/performance tests)

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Nitrogen. llvmdev.

Checklist

  • NA. This PR is up to date with current the current state of 'develop'
  • NA. 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 Feb 11, 2021
@ye-luo ye-luo merged commit 34cdb3b into QMCPACK:develop Feb 12, 2021
@correaa
Copy link
Contributor

correaa commented Feb 16, 2021

@markdewing @prckent

Please add numdiff -a 0.0001 ../build/src/QMCTools/ppconvert/test/Oxygen/O.BFD.xml ../tests/solids/monoO_1x1x1_pp/O.BFD.xml to the tests, (with dependency or as part of ppconvert_does_not_crash). I don't know how to do this.

Also consider making -DBUILD_PPCONVERT=1 the default for cmake (if not already).

@prckent
Copy link
Contributor Author

prckent commented Feb 16, 2021

@correaa copy your request to an issue please so that we don't lose it

We can't make BUILD_PPCONVERT the default until we have verified that it builds with all the usual compilers. A simple step, but it needs to be done. We can do that after your changes. It is an optional build in part because we can't risk breaking the main build and in part because a long while back it had no cmake configuration.

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.

None yet

3 participants