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

let internal easyblock not create a log file in QuantumESPRESSO easyblock #3505

Merged

Conversation

Thyre
Copy link
Contributor

@Thyre Thyre commented Nov 12, 2024

This fixes the increased disk usage when running the EasyConfig test suite as log files would stay open.
See easybuilders/easybuild-easyconfigs#21841 for more information.

Requires:

Fixes:


Old Notes:

  • During the EasyConfig test suite, some log files will be left over until the suite has finished. However, these files are now only ~1.1KB in size instead of steadily increasing in size.
  • It's hard to test the changes in the CI.
  • We need to check if these changes affect normal installation logs. Until then, marked as draft.

@Thyre Thyre marked this pull request as ready for review November 13, 2024 14:21
@Thyre
Copy link
Contributor Author

Thyre commented Nov 13, 2024

Logs of normal build seem to be still working.
This is more or less a quick fix.

My idea for a more robust fix would be:

If we change the EasyBlock in framework to optionally accept a logger and only create a new one if one was not passed, we could pass in the logger from Bundle and QuantumESPRESSO to the inner EasyBlocks. This logger can then be closed with the normal close_log call.
This should also fix the leftover files in this PR. If we also want to make sure that this passed logger is not closed accidentally in the inner EasyBlock (which I would prefer), handling QuantumESPRESSO might get a bit more complicated due to the way the EasyBlock is written (basically replacing itself via __getattribute__). This can also be solved, but I have no good solution for that yet.

I will take a look and update this PR accordingly.

@Thyre Thyre changed the title Draft: Close QuantumESPRESSO logger before instantiating internal EasyBlock Close QuantumESPRESSO logger before instantiating internal EasyBlock Nov 13, 2024
@Thyre Thyre marked this pull request as draft November 14, 2024 10:04
@Thyre Thyre force-pushed the close-quantumespresso-logger-early branch from f0ffe6d to 579e6bd Compare November 14, 2024 12:34
@Thyre
Copy link
Contributor Author

Thyre commented Nov 14, 2024

CI fails as expected, due to changes in framework.

@Thyre
Copy link
Contributor Author

Thyre commented Nov 14, 2024

Found issues with the implementation in framework. Changed the implementation and will try again.

@Thyre Thyre force-pushed the close-quantumespresso-logger-early branch from 579e6bd to 3736885 Compare November 14, 2024 14:21
@Thyre
Copy link
Contributor Author

Thyre commented Nov 14, 2024

New changes in framework work together with this PR. The EasyConfig test suite fails due to GROMACS, but this is not related to this PR. Will leave this as Draft until the Bundle PR, initially introduced in #3472, is also ready and I'm sure that everything works for sure.

@Thyre Thyre changed the title Close QuantumESPRESSO logger before instantiating internal EasyBlock QuantumESPRESSO: Let internal EasyBlock not create a log file Nov 15, 2024
This fixes the increased memory usage when running the EasyConfig test suite
as log files would stay open.

Signed-off-by: Jan André Reuter <[email protected]>
@Thyre Thyre force-pushed the close-quantumespresso-logger-early branch from 3736885 to 2e61705 Compare November 15, 2024 08:04
@Thyre Thyre marked this pull request as ready for review November 15, 2024 08:45
@Thyre
Copy link
Contributor Author

Thyre commented Nov 15, 2024

Test report by @Thyre

Overview of tested easyconfigs (in order)

  • SUCCESS gompi-2024a.eb
  • SUCCESS HDF5-1.14.5-gompi-2024a.eb
  • SUCCESS FFTW.MPI-3.3.10-gompi-2024a.eb
  • SUCCESS ScaLAPACK-2.2.0-gompi-2024a-fb.eb
  • SUCCESS foss-2024a.eb
  • SUCCESS ELPA-2024.05.001-foss-2024a.eb
  • SUCCESS QuantumESPRESSO-7.3.1-foss-2024a.eb

Build succeeded for 7 out of 7 (1 easyconfigs in total)
datenlager - Linux Ubuntu 24.04, x86_64, AMD Ryzen 7 3700X 8-Core Processor, Python 3.12.3
See https://gist.github.com/Thyre/e0e5e44264f991f13c45644b5b79ed65 for a full test report.

@Thyre
Copy link
Contributor Author

Thyre commented Dec 4, 2024

@boegelbot please test @ jsc-zen3
EB_ARGS="QuantumESPRESSO-7.3.1-foss-2024a.eb"

@boegelbot
Copy link

@Thyre: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3505 EB_ARGS="QuantumESPRESSO-7.3.1-foss-2024a.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3505 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 5378

Test results coming soon (I hope)...

- notification for comment with ID 2516322942 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS QuantumESPRESSO-7.3.1-foss-2024a.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.19
See https://gist.github.com/boegelbot/6b60f1e3f030b95fa9f0ea869d7648ff for a full test report.

@boegel
Copy link
Member

boegel commented Dec 4, 2024

CI tests re-triggered now that easybuilders/easybuild-framework#4707 is merged...

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel added the bug fix label Dec 4, 2024
@boegel boegel added this to the release after 4.9.4 milestone Dec 4, 2024
@boegel boegel changed the title QuantumESPRESSO: Let internal EasyBlock not create a log file let internal easyblock not create a log file in QuantumESPRESSO easyblock Dec 4, 2024
@boegel
Copy link
Member

boegel commented Dec 18, 2024

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS QuantumESPRESSO-7.3.1-foss-2023a.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3105.skitty.os - Linux RHEL 9.4, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.9.18
See https://gist.github.com/boegel/e58943a27a057052124ba13f5c51fd09 for a full test report.

@boegel boegel merged commit 42f7994 into easybuilders:develop Dec 18, 2024
41 checks passed
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.

3 participants