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

Fixed debug compilation on Lassen #1368

Merged
merged 2 commits into from
Mar 23, 2021
Merged

Conversation

francoishamon
Copy link
Contributor

@francoishamon francoishamon commented Mar 23, 2021

This PR moves the implementation of the two MultiFluidPVTPackageWrapper::compute functions to MultiFluidPVTPackageWrapper.hpp in order to fix #1366.

Resolves #1366

@francoishamon francoishamon requested a review from corbett5 March 23, 2021 02:21
@francoishamon francoishamon added the type: bug Something isn't working label Mar 23, 2021
Comment on lines 26 to 28
// For a reason that I don't understand, adding this include
// requires adding PVTPackage as a dependent of the physicsSolvers
#include "pvt/pvt.hpp"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving the two compute functions to the hpp file requires including pvt/pvt.hpp in MultiFluidPVTPackageWrapper.hpp. If I don't add PVTPackage as a dependency of physicsSolvers, then I get the error:

MultiFluidPVTPackageWrapper.hpp:25:10: fatal error: 'pvt/pvt.hpp' file not found

Apparently someone had run into the same issue before (see the deleted comment on GitHub). I spent some time trying to fix this but could not find a good solution. If someone knows how to make this works without making PVTPackage a dependency of the physicsSolvers, let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bmhan12 this isn't going to make your life any easier. Sorry.

Copy link
Member

Choose a reason for hiding this comment

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

Can't PVTPackage be a dependency of constitutive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PVTPackage was already a dependency of constitutive:
https://github.com/GEOSX/GEOSX/blob/848d9e5e180d0bb90309704c4f8b300daa2c10be/src/coreComponents/constitutive/CMakeLists.txt#L119-L133
and this was enough when pvt/pvt.hpp was included in MultiFluidPVTPackageWrapper.cpp. Now that pvt/pvt.hpp has to be included in MultiFluidPVTPackageWrapper.hpp, it is not enough anymore, and the included file pvt/pvt.hpp cannot be found (unless PVTPackage is a also dependency of physicsSolvers).

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't PVTPackage be a dependency of constitutive?

It already is, but constitutive isn't a dependency of physicsSolvers:
https://github.com/GEOSX/GEOSX/blob/848d9e5e180d0bb90309704c4f8b300daa2c10be/src/coreComponents/physicsSolvers/CMakeLists.txt#L111-L115

Copy link
Contributor Author

@francoishamon francoishamon Mar 23, 2021

Choose a reason for hiding this comment

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

In the last commit I made physicsSolvers depend on constitutive (and as before, constitutive itself depends on PVTPackage).

@francoishamon francoishamon added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Mar 23, 2021
@francoishamon francoishamon merged commit 918a9d0 into develop Mar 23, 2021
@francoishamon francoishamon deleted the bugfix/hamon/issue1366 branch March 23, 2021 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Develop doesn't build on Lassen with Clang in debug
4 participants