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

Fix linking with APMIDG for Intel GPUs #561

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

masterleinad
Copy link
Contributor

Description

Without this pull request Variorum doesn't link against APMIDG resulting in linker errors. This pull request is also fixing some compiler warnings. In particular, the str array is never NULL making one check obsolete. The success of writing into this array via fgets is already checked above.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/architecture support (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Build/CI update

How Has This Been Tested?

Building and testing on ALCF's sunspot testbed.

Checklist:

  • I have run ./scripts/check-code-format.sh and confirm my code code follows the style guidelines of variorum
  • I have added comments in my code
  • My changes generate no new warnings (build with -DENABLE_WARNINGS=ON)
  • New and existing unit tests pass with my changes

Thank you for taking the time to contribute to Variorum!

@tpatki
Copy link
Member

tpatki commented Jun 27, 2024

Hi @masterleinad ! Thank you for this, we don't have access to Intel GPUs so we can't test this at our end (Kaz @kazutomo tested it for us before).

  • Can you run the scripts/check-code-format.sh script and address the code formatting issue? Let me know if you have trouble setting this up at your end, I can take care of it as well.
  • Can you attach a sunspot host-config file as well for other users at ANL?

@masterleinad
Copy link
Contributor Author

Can you run the scripts/check-code-format.sh script and address the code formatting issue? Let me know if you have trouble setting this up at your end, I can take care of it as well.

I tried doing that but ran into

> ./scripts/check-code-format.sh 
Checking format of cpp/c/h files using astyle
Invalid command line options:
	max-instatement-indent=40
	break-closing-brackets
	add-brackets
For help on options type 'astyle -h'
Artistic Style has terminated

'rstfmt' not found: this package can be installed using pip

Do I need a particular version?

@tpatki
Copy link
Member

tpatki commented Jun 27, 2024

Hi @masterleinad Thanks for the prompt reply! I don't think you need a particular version. I can format this for you if it doesn't work easily at your end, let me know accordingly.

@masterleinad masterleinad force-pushed the fix_compiling_intel_apmidg branch from 3984992 to 57e7dfc Compare June 27, 2024 21:51
@masterleinad
Copy link
Contributor Author

Hi @masterleinad Thanks for the prompt reply! I don't think you need a particular version. I can format this for you if it doesn't work easily at your end, let me know accordingly.

Using 3.1 worked now.

@masterleinad
Copy link
Contributor Author

Can you attach a sunspot host-config file as well for other users at ANL?

I needed to install jansson and apmidg myself. I'm not quite sure what the path forward would be.

@tpatki
Copy link
Member

tpatki commented Jun 27, 2024

Ah, I see. Maybe a template or example host-config then, that people can copy and add the path to APMIDG. Jansson is already a Variorum dependency so folks would know how to set that up.

@tpatki tpatki merged commit 87d3861 into LLNL:dev Jun 27, 2024
9 checks passed
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.

2 participants