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

Update CircleCI to use both gfortran and ifort #831

Merged
merged 25 commits into from
Jul 8, 2021
Merged

Update CircleCI to use both gfortran and ifort #831

merged 25 commits into from
Jul 8, 2021

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented May 7, 2021

This PR enables both gfortran and ifort builds of MAPL and GEOSgcm

Closes #830

@mathomp4 mathomp4 added the 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. label May 7, 2021
@mathomp4
Copy link
Member Author

mathomp4 commented May 7, 2021

Time to ask CircleCI Forum for help...

@mathomp4 mathomp4 added 0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) and removed 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. labels May 10, 2021
@mathomp4
Copy link
Member Author

Woooo! I think I got it. We'll find out...

@mathomp4
Copy link
Member Author

I await the inevitable discussion with @tclune on the commits I made to MAPL_MemUtils.F90, but it works!

@mathomp4
Copy link
Member Author

Timings from CircleCI:

MAPL Build Test:

  • Intel: 4m 25s
  • GNU: 8m 43s

GEOSgcm Build:

  • Intel: 9m 13s
  • GNU: 15m 33s

Go Intel, I guess!

@mathomp4 mathomp4 marked this pull request as ready for review May 10, 2021 19:16
@mathomp4 mathomp4 requested a review from a team as a code owner May 10, 2021 19:16
@mathomp4 mathomp4 added the 🚫 Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label May 10, 2021
@mathomp4
Copy link
Member Author

Throwing on a blocker until @tclune can see and comment.

@mathomp4
Copy link
Member Author

Closes #830

@mathomp4 mathomp4 linked an issue May 11, 2021 that may be closed by this pull request
Update CircleCI to do Intel Builds #830
Closed
@mathomp4
Copy link
Member Author

Some cost analysis of this PR using 40 credits/min on xlarge:

  • Build and Test MAPL
    • Intel: 4m 1s → 161 credits
    • GNU: 8m 50s → 353 credits
  • Build GEOSgcm
    • Intel: 8m 55s → 356 credits
    • GNU: 17m 51s → 714 credits
  • Make FV3 Exp
    • Intel: 1m 42s → 88 credits
    • GNU: 39s → 26 credits
  • Run FV3 Exp
    • Intel: 2m 40s → 107 credits
    • GNU: 58s → 39 credits

The total here is:

  • Intel: 712
  • GNU: 1132
  • Total: 1844

CircleCI reports 1761 so not too far off.

@mathomp4
Copy link
Member Author

mathomp4 commented May 11, 2021 •
edited
Loading

Okay. Just did a test using large instead of xlarge and found:

  • Build and Test MAPL
    • Intel: 4m 11s → 84 credits
    • GNU: 9m 22s → 187 credits
  • Build GEOSgcm
    • Intel: 8m 55s → 178 credits
    • GNU: 17m 13s → 344 credits

CircleCI reports 796 credits used, again, about right.

Well, that's obviously better. (I also turned off the FV3 tests as they are probably not that necessary.)

@mathomp4 mathomp4 mentioned this pull request May 11, 2021
Merged
@mathomp4 mathomp4 marked this pull request as draft May 12, 2021 13:56
@mathomp4 mathomp4 added 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. and removed 0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) 🚫 Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs labels Jul 6, 2021
@mathomp4 mathomp4 requested a review from tclune July 6, 2021 17:49
@mathomp4
Copy link
Member Author

mathomp4 commented Jul 6, 2021

Getting close to undrafting. Frankly, I don't think we miss anything by not building/running GEOSfvdycore, and I think the ability to build/test both GNU and Intel is more important. I'll see what @tclune says.

@mathomp4
Copy link
Member Author

mathomp4 commented Jul 6, 2021

Decided to try adding the GEOSadas to the CI. But it looks like we are missing something for MKL in the Docker image. Let's figure that out.

@mathomp4
Copy link
Member Author

mathomp4 commented Jul 6, 2021

So, it turns out we can't quite do GEOSadas with MAPL yet. The reason is that GEOSadas is now "behind" GEOSgcm in a crucial way: Baselibs. So, time to make a PR...

@mathomp4
Copy link
Member Author

mathomp4 commented Jul 6, 2021

PR to GEOSadas submitted: GEOS-ESM/GEOSadas#103

@mathomp4 mathomp4 marked this pull request as ready for review July 8, 2021 15:39
@mathomp4 mathomp4 merged commit 996488f into develop Jul 8, 2021
@mathomp4 mathomp4 deleted the feature/mathomp4/enable-all-ifort-mapl branch July 8, 2021 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tclune tclune tclune approved these changes

Assignees
No one assigned
Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update CircleCI to do Intel Builds
2 participants
@mathomp4 @tclune