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

Add AMD Milan Support #304

Closed
wants to merge 3 commits into from
Closed

Add AMD Milan Support #304

wants to merge 3 commits into from

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented Jan 4, 2023 •
edited
Loading

This PR adds support for AMD Milan. Testing at NAS shows that:

elseif (${proc_description} MATCHES "EPYC 7..2")

matches the Romes and that:

elseif (${proc_description} MATCHES "EPYC 7..3")

matches the Milan processors on AWS.

We mainly do this to use -znver3 on AWS.

UPDATE: Turns out tests on the Milans at NCCS show there is no difference between -znver2 and -znver3. Same speed, zero-diff. Thus we do not add complication to ESMA_cmake and close this PR.

@mathomp4 mathomp4 added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Jan 4, 2023
@mathomp4 mathomp4 requested a review from a team as a code owner January 4, 2023 14:34
@mathomp4 mathomp4 self-assigned this Jan 4, 2023
tclune
tclune previously approved these changes Jan 4, 2023
View reviewed changes
@mathomp4 mathomp4 added the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Jan 4, 2023
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

@mathomp4
Copy link
Member Author

mathomp4 commented Jan 4, 2023

I'm going to block this until I can actually test on Milan. Probably good not to break things. 😄

@mathomp4
Copy link
Member Author

mathomp4 commented Jan 5, 2023

Well, testing with the Bill Branch shows it crashes with any optimization it seems. znver2, znver3, native...

I'll try stock GEOSgcm...

@mathomp4 mathomp4 closed this Jan 19, 2024
@mathomp4 mathomp4 deleted the feature/mathomp4/add-milan-support branch January 19, 2024 14:05
@mathomp4 mathomp4 restored the feature/mathomp4/add-milan-support branch January 19, 2024 14:05
@mathomp4 mathomp4 reopened this Jan 19, 2024
@github-actions GitHub Actions
Copy link

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

@github-actions GitHub Actions
Copy link

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

@mathomp4 mathomp4 closed this Jan 19, 2024
@mathomp4 mathomp4 deleted the feature/mathomp4/add-milan-support branch January 19, 2024 17:52
@github-actions GitHub Actions
Copy link

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tclune tclune tclune left review comments

Assignees

@mathomp4 mathomp4

Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mathomp4 @tclune