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

Revert pio1 cmake path to point into pio2 #1480

Closed
wants to merge 1 commit into from

Conversation

agsalin
Copy link
Contributor

@agsalin agsalin commented May 4, 2017

Add back in the pointing of pio1 into pio2/cmake, for now.

The CMakelists.txt file for pio1/pio used to point into pio2/cmake for functions like FindNetCDF.cmake.
commit "Dont point to pio2 cmake modules by default"
PR#1352

This was changed in pio1/pio to point to "${CMAKE_CURRENT_SOURCE_DIR}/cmake" which doesn't exist.

Without a FindNetCDF, the ACMCE build of the HOMME test to fail.

Changing this back to HOMME build works again until a better fix is found or we switch to pio2.

Test suite: Discovered in ACME -- just typed it in here without running tests.
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes: ACME integration of CIME master as of May 2

Add back in the pointing of pio1 into pio2/cmake, for now.

The CMakelists.txt file for pio1/pio used to point into pio2/cmake for functions like FindNetCDF.cmake.
commit "Dont point to pio2 cmake modules by default"
PR#1352

This was changed in pio1/pio to point to "${CMAKE_CURRENT_SOURCE_DIR}/cmake" which doesn't exist.

Without a FindNetCDF, the ACMCE build of the HOMME test to fail. 

Changing this back to HOMME build works again until a better fix is found or we switch to pio2.
@jedwards4b
Copy link
Contributor

Andy,
Please use the template for the initial PR message and remove/replace text in [].

Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

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

I think that Jayesh's intent here was that CIME should be passing in USER_CMAKE_MODULE_PATH to point to the pio2/cmake and it shouldn't be explicitly listed here.

@agsalin
Copy link
Contributor Author

agsalin commented May 4, 2017

A fix that sets USER_CMAKE_MODULE_PATH sounds preferable. Hopefully @jayeshkrishna can figure out where to do this. This is the ACME test to verify: HOMME_P24.f19_g16_rx1.A

@jayeshkrishna
Copy link
Contributor

The right fix for HOMME tests to work is in E3SM-Project/E3SM#1480 . Specifically commit - E3SM-Project/E3SM@443aa5e

Copy link
Contributor

@jayeshkrishna jayeshkrishna left a comment

Choose a reason for hiding this comment

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

pio1 cmake configure files should not have been pointing to pio2 cmake modules (pio1 is independent of pio2).

@agsalin
Copy link
Contributor Author

agsalin commented May 4, 2017

OK, that sounds good. We're having trouble getting the sequence correct and finding the same bug twice.

One thing that still looks wrong to me is pointing to the pio1/pio/cmake dir that doesn't exist, but that doesn't effect me.

@agsalin agsalin closed this May 4, 2017
jgfouca pushed a commit that referenced this pull request Jun 2, 2017
Get HOMME tests to build on Mira. The following merge,

* Brings in the latest version of pio1 (includes changes to pio1
  cmake configure files required for this issue)
* Updates HOMME cmake configure files to include pio2 cmake
  modules

See Issue #1411

[BFB]

* jayeshkrishna/mira_homme_test_update_pio1:
  Add path to pio2 cmake modules for HOMME tests
  Updating PIO1 to latest master
jgfouca pushed a commit that referenced this pull request Feb 23, 2018
Get HOMME tests to build on Mira. The following merge,

* Brings in the latest version of pio1 (includes changes to pio1
  cmake configure files required for this issue)
* Updates HOMME cmake configure files to include pio2 cmake
  modules

See Issue #1411

[BFB]

* jayeshkrishna/mira_homme_test_update_pio1:
  Add path to pio2 cmake modules for HOMME tests
  Updating PIO1 to latest master
jgfouca pushed a commit that referenced this pull request Mar 13, 2018
Get HOMME tests to build on Mira. The following merge,

* Brings in the latest version of pio1 (includes changes to pio1
  cmake configure files required for this issue)
* Updates HOMME cmake configure files to include pio2 cmake
  modules

See Issue #1411

[BFB]

* jayeshkrishna/mira_homme_test_update_pio1:
  Add path to pio2 cmake modules for HOMME tests
  Updating PIO1 to latest master
@agsalin agsalin deleted the agsalin/revert-pio1-cmake-point-into-pio2 branch March 14, 2018 16:47
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.

4 participants