search

firemodels / fds

Fire Dynamics Simulator
https://pages.nist.gov/fds-smv/
Other
673 stars 627 forks source link

github-actions: update oneapi for windows. #13721

Closed marcfehling closed 3 weeks ago

marcfehling commented 3 weeks ago

That doesn't work yet. I will try figure it out on my fork and will re-open this PR.

cxp484 commented 3 weeks ago

Hi Marc, I think similar to linux.yml you need to add this line in windows action script, as the makefile by default build with ifort. But, if we have set the INTEL_IFORT env variable then it build with that. export INTEL_IFORT=ifx

For windows it would be probably: set INTEL_IFORT=ifx

marcfehling commented 3 weeks ago

I tried to set INTEL_IFORT=ifx, but the make_fds.bat script was still using ifort. Example: https://github.com/marcfehling/fds/actions/runs/11743376784/workflow#L88

I am trying now to specify I_IFORT=ifx by default in the Makefile. Proposal: https://github.com/marcfehling/fds/blob/cd85d8b9ebd8d437a878fa3a2e7c19814ebadcd3/Build/makefile#L16

I'll open up this PR once I found a working solution.

cxp484 commented 3 weeks ago

Yes. I found that the Build/Scripts/setup_intel_compilers.bat set the INTEL_IFORT back to ifort. So our explicit setting never get enforced. I think we need to modify the setup_intel_compilers.bat file. Let me talk to @rmcdermo and @mcgratta to see what should be the best way to do it, because we probably still want ifort to be the default for the time being. For github actions we need ifx to be default.

marcfehling commented 3 weeks ago

See also #13715.

ifort has been removed from oneapi with the 2025.0 release. ifx has already been around for the last few releases.

I would suggest to make fds "future-proof" and switch to ifx everywhere independent of github actions, i.e., adjust all occurrences of

fds:$ grep -r "ifort"
rmcdermo commented 3 weeks ago

@marcfehling The problem is that we are finding ifx to be about 20% slower than ifort. So, we cannot completely switch over right now. @mcgratta has researched this more and maybe he can comment. But we do not know what Intel's future is, so hard to future-proof FDS in this scenario.

cxp484 commented 3 weeks ago

You're correct that we will eventually need to make the transition. However, we've found that the FDS executables built with ifx are approximately 20% slower than those built with ifort, which is a significant difference. For this reason, we still support ifort. That said, over time, we plan to fully transition to ifx.

I submitted a changed to Windows actions. Now it looks like working.