Skip to content

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Aug 2, 2024

(created using eb --new-pr)

During sanity-check-only the prepare_step is skipped which sets up the toolchain including toolchain and environment variables such as $CC

Without that step (i.e. in sanity-check-only mode) os.getenv('CC') will fail because the variable is not set. Or even worse: It is set but to a wrong value.

Additionally ec['parallel'] is also not set yielding a mpirun -n None which of course fails.

This PR adds the prepare_step and set_parallel call to initialize those. It also uses a temporary directory for the test binary because the builddir also doesn't exist.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS impi-2019.7.217-iccifort-2020.1.217.eb
  • SUCCESS impi-2019.7.217-iccifortcuda-2020a.eb
  • SUCCESS impi-2019.9.304-iccifort-2020.4.304.eb
  • SUCCESS impi-2019.9.304-iccifortcuda-2020b.eb
  • SUCCESS impi-2021.10.0-intel-compilers-2023.2.1.eb
  • SUCCESS impi-2021.11.0-intel-compilers-2024.0.0.eb
  • SUCCESS impi-2021.13.0-intel-compilers-2024.2.0.eb
  • SUCCESS impi-2021.2.0-intel-compilers-2021.2.0.eb
  • SUCCESS impi-2021.4.0-intel-compilers-2021.4.0.eb
  • SUCCESS impi-2021.5.0-intel-compilers-2022.0.1.eb
  • SUCCESS impi-2021.6.0-intel-compilers-2022.1.0.eb
  • SUCCESS impi-2021.7.1-intel-compilers-2022.2.1.eb
  • SUCCESS impi-2021.8.0-intel-compilers-2023.0.0.eb
  • SUCCESS impi-2021.9.0-intel-compilers-2023.1.0.eb

Build succeeded for 14 out of 14 (14 easyconfigs in total)
n1625 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (icelake), Python 3.8.17
See https://gist.github.com/Flamefire/a68a4257fd9a053a76a855671f68157e for a full test report.

Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Contributor

Was a bit too quick here... If we use tempfile.mkdtemp outside of builddir we need to delete it afterwards.
So should unconditionally remove the basedir of impi_testexe after.

@Flamefire
Copy link
Contributor Author

Was a bit too quick here... If we use tempfile.mkdtemp outside of builddir we need to delete it afterwards. So should unconditionally remove the basedir of impi_testexe after.

The base directory for that is easybuild-tmp which IIRC will be deleted after the build or at least after the eb run. Isn't that enough?

If it isn't we should only delete the tmp dir, not the builddir as one might want to inspect that as before via --disable-cleanup-build

@akesandgren
Copy link
Contributor

Ah yes, forgot that TMPDIR is set to eb controlled space. Blaming first week after vacation for brain not being fully engaged.

@akesandgren
Copy link
Contributor

Going in, thanks @Flamefire!

@akesandgren akesandgren merged commit 9a69e33 into easybuilders:develop Aug 13, 2024
@Flamefire Flamefire deleted the 20240802133335_new_pr_impi branch August 13, 2024 10:27
@boegel boegel changed the title fix sanity-check-only for impi fix --sanity-check-only for impi Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants