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

QuantumESPRESSO: Let internal EasyBlock not create a log file #3505

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

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.

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.

1 participant