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.

Sign up for GitHub

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

Jump to bottom

Bugfix/sytuan/pangea build #1253

Merged
merged 1 commit into from
Jan 27, 2021
Merged

Bugfix/sytuan/pangea build #1253

merged 1 commit into from
Jan 27, 2021

Conversation

sytuannguyen
Copy link
Contributor

No description provided.

@sytuannguyen
Copy link
Contributor Author

sytuannguyen commented Dec 18, 2020 •
edited
Loading

The build on Pangea is succeeded with these modifications suggested by Thomas.

@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from 9c5f6e6 to d61194c Compare December 18, 2020 13:58
@rrsettgast rrsettgast requested a review from bmhan12 December 21, 2020 19:35
@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from d61194c to 6e0beda Compare December 23, 2020 14:15
@rrsettgast
Copy link
Member

@TotoGaz This one is yours to approve and merge.

herve-gross
herve-gross reviewed Jan 4, 2021
View reviewed changes
src/cmake/thirdparty/SetupGeosxThirdParty.cmake Outdated
@@ -197,6 +202,7 @@ endif()
################################
# Umpire
################################
set(UMPIRE_DIR ${GEOSX_TPL_DIR}/chai)
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be umpire?

Copy link
Contributor Author

@sytuannguyen sytuannguyen Jan 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I observed (see e.g. in host-configs/tpls.cmake) that both UMPIRE_DIR and CHAI_DIR are set to ${GEOSX_TPL_DIR}/chai

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes indeed

if(EXISTS ${GEOSX_TPL_DIR}/chai)
  set(UMPIRE_DIR ${GEOSX_TPL_DIR}/chai CACHE PATH "" FORCE)
  set(CHAI_DIR ${GEOSX_TPL_DIR}/chai CACHE PATH "" FORCE)
endif()

I would bet this is a copy paste forgetfulness. Do you confirm @corbett5 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@TotoGaz the way we build CHAI currently, it also builds Umpire for us and it is located in CHAI_DIR. When we build with Spack Umpire is created in a separate directory.

TotoGaz
TotoGaz reviewed Jan 4, 2021
View reviewed changes
Copy link
Contributor

@TotoGaz TotoGaz left a comment

Choose a reason for hiding this comment

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

Can you investigate why your variable are not defined in tpls.cmake?

@TotoGaz TotoGaz assigned sytuannguyen and unassigned TotoGaz Jan 4, 2021
TotoGaz
TotoGaz reviewed Jan 19, 2021
View reviewed changes
Copy link
Contributor

@TotoGaz TotoGaz left a comment

Choose a reason for hiding this comment

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

Can you fix your conflict with the integrated tests and then we can merge this?

@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from c9db247 to 5087282 Compare January 20, 2021 08:47
@sytuannguyen sytuannguyen requested review from rrsettgast, corbett5, TotoGaz and herve-gross and removed request for rrsettgast, bmhan12 and herve-gross January 26, 2021 07:48
@sytuannguyen sytuannguyen requested review from rrsettgast and removed request for rrsettgast and herve-gross January 26, 2021 07:53
@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from 5087282 to f396f9f Compare January 26, 2021 12:30
@TotoGaz
Copy link
Contributor

TotoGaz commented Jan 26, 2021

@sytuannguyen Obviously an issue with your PTP submodule.
Please correct it and ping me I'll merge this PR.

@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from f396f9f to ac0e2d7 Compare January 27, 2021 08:28
@sytuannguyen sytuannguyen force-pushed the bugfix/sytuan/PangeaBuild branch from ac0e2d7 to 66dd183 Compare January 27, 2021 08:38
@sytuannguyen sytuannguyen reopened this Jan 27, 2021
@sytuannguyen
Copy link
Contributor Author

sytuannguyen commented Jan 27, 2021 •
edited
Loading

@TotoGaz It's OK now, please merge this PR

@TotoGaz
Copy link
Contributor

TotoGaz commented Jan 27, 2021

@rrsettgast I'd like to merge this for the moment. Your NOK was about the tpls.cmake which has been corrected. Is it OK for you?

@rrsettgast rrsettgast merged commit a5f3f10 into develop Jan 27, 2021
@rrsettgast rrsettgast deleted the bugfix/sytuan/PangeaBuild branch January 27, 2021 17:48
sytuannguyen added a commit that referenced this pull request Feb 2, 2021
@sytuannguyen
correct host configs for Pangea build (#1253)
b89c7d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@herve-gross herve-gross herve-gross left review comments

@rrsettgast rrsettgast rrsettgast approved these changes

@TotoGaz TotoGaz TotoGaz approved these changes

@corbett5 corbett5 Awaiting requested review from corbett5

Assignees

@sytuannguyen sytuannguyen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sytuannguyen @rrsettgast @TotoGaz @corbett5 @herve-gross