Updated Intel config to work with stable and latest Intel versions

[sseraj]

Purpose

The Intel version changes on our Docker images (mdolab/docker#266) requires changing the Intel config files to use different compilers depending on the Intel version. This PR should be used to work out the necessary changes before applying them to all of our repos.

Expected time until merged

Before mdolab/docker#266 is merged

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

Testing

Checklist

• I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
• I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.48%. Comparing base (1bc1854) to head (c0b9ee8).

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #87   +/-   ##
=======================================
  Coverage   79.48%   79.48%           
=======================================
  Files           4        4           
  Lines         424      424           
=======================================
  Hits          337      337           
  Misses         87       87           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[A-CGray]

@sseraj is this ready for review?

[sseraj]

Yes, it's ready for review. I kept it as a draft to avoid merging before finalizing the config, but I'll mark it ready for review

[eirikurj]

The changes look good, but I have only tested this on an old Intel container with icc/ifort only. Ideally, I want to test this on the Intel docker containers in https://github.com/mdolab/docker/pull/266 with the relevant compilers installed. @sseraj did you by any chance test both latest and stable?

[sseraj]

I only tested on latest, but with the correct and inverted logic. Between the two of us, we covered both images anyway.

[A-CGray]

@sseraj this logic doesn't seem to work, when icc doesn't exist it still tries to use the old executables. What does seem to work is replacing the two ICC_EXISTS lines with ifneq ($(shell command -v icc 2> /dev/null;),). I tested this both on NAS (which has icc) and stampede3 (that doesn't)

[eirikurj]

@A-CGray I think the previous logic should work. The issue is that the inline comment that was added is messing with the ifdef evaluation due to the whitespace after the parenthesis (should have known this). Removing it, or more safely, moving the comment above or below should be sufficient to make this work, i.e., change,

ICC_EXISTS := $(shell command -v icc;) # Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail

to

ICC_EXISTS := $(shell command -v icc;)# Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail

or better

# Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail
ICC_EXISTS := $(shell command -v icc;)

Aside from all this, the single line comparison expression you suggested is also fine and should do the same. I dont mind either approach. However, I dont think we want the 2> /dev/null and think we should remove that, as it is a helpful output if for some reason command fails (although very unlikely) and we hit the else block we did not expect.

[eirikurj]

Added a PR here for pyfriction based on your suggestion, without the stderr redirect. I kept the comment as this is a subtle thing.

[A-CGray]

@A-CGray I think the previous logic should work. The issue is that the inline comment that was added is messing with the ifdef evaluation due to the whitespace after the parenthesis (should have known this). Removing it, or more safely, moving the comment above or below should be sufficient to make this work, i.e., change,

ICC_EXISTS := $(shell command -v icc;) # Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail

to

ICC_EXISTS := $(shell command -v icc;)# Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail

or better

# Note that ";" is there to avoid make shell optimization, otherwise the shell command may fail
ICC_EXISTS := $(shell command -v icc;)

Aside from all this, the single line comparison expression you suggested is also fine and should do the same. I dont mind either approach. However, I dont think we want the 2> /dev/null and think we should remove that, as it is a helpful output if for some reason command fails (although very unlikely) and we hit the else block we did not expect.

Oh nice, I like the third suggestion, better than my ifneq statement as its more readable