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

Update README.md to add windows docs #48

Merged
merged 17 commits into from
Nov 22, 2023
Merged

Conversation

mondus
Copy link
Contributor

@mondus mondus commented Oct 16, 2023

Adds details of

  • How to install Intel Fortran in Windows
  • Details on obtaining the CMAKE_PREFIX_PATH
  • Cross platform build and install commands

@mondus mondus requested a review from jatkinson1000 October 16, 2023 14:29
Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

Thanks @mondus a couple of small tweaks and a question about the install command.
I've forwarded it to the user and will see how she gets on - using cmake --build rather than make may be important!

README.md Outdated Show resolved Hide resolved
@jatkinson1000
Copy link
Member

This is WIP with #50
Once that is resolved with the user we will update here accordingly and then merging will close #50

@jatkinson1000 jatkinson1000 linked an issue Oct 19, 2023 that may be closed by this pull request
@jatkinson1000 jatkinson1000 mentioned this pull request Nov 18, 2023
7 tasks
@jatkinson1000
Copy link
Member

I think this is good to go now (correspondent from #50 has gone quiet, but it works for us).
Since I have done a lot of this PR I will let @TomMelt do a final review for approval.

Copy link
Member

@TomMelt TomMelt left a comment

Choose a reason for hiding this comment

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

looks good to me. Just a couple of questions

examples/2_ResNet18/README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@TomMelt
Copy link
Member

TomMelt commented Nov 21, 2023

just added a note about loading intel compilers. If @mondus is happy then I think we can merge it in.

@jatkinson1000
Copy link
Member

just added a note about loading intel compilers. If @mondus is happy then I think we can merge it in.

@TomMelt Would it be worth extending that by telling users that this can usually be done by running C:\Program Files (x86)\Intel\oneAPI\setvars from Cmd?

Copy link
Contributor Author

@mondus mondus left a comment

Choose a reason for hiding this comment

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

Tested in Windows. All good. Happy to approve. One minor suggestion comment to add a link to Intel guide on setvars.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Paul Richmond <p.richmond@sheffield.ac.uk>
@jatkinson1000
Copy link
Member

Thanks @mondus, that also addresses my comment above but in an even safer/better way than suggested, thanks.
I'll approve and merge now (@mondus has confirmed offline that he is happy with the PR).

Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

Thanks all who contributed here, I'm now happy to go ahead so will merge in a moment.

@jatkinson1000 jatkinson1000 dismissed TomMelt’s stale review November 22, 2023 11:00

Changes have been made as appropriate.

@jatkinson1000 jatkinson1000 merged commit 35d1747 into main Nov 22, 2023
1 check passed
@jatkinson1000 jatkinson1000 deleted the windows-build-instructions branch November 22, 2023 11:01
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.

Issues building on Windows
3 participants