Skip to content

Remove vresutils#662

Merged
fneum merged 6 commits intomasterfrom
remove-vresutils
May 12, 2023
Merged

Remove vresutils#662
fneum merged 6 commits intomasterfrom
remove-vresutils

Conversation

@virio-andreyana
Copy link
Copy Markdown
Collaborator

Changes proposed in this Pull Request

Remove the depreciated vresutils from pypsa-eur.

Affected scripts

  • add_electricity.py
  • prepare_sector_network.py
  • solve_network.py
  • solve_operations_network.py

Checklist

  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in all of config.default.yaml.
  • Changes in configuration options are also documented in doc/configtables/*.csv.
  • A release note doc/release_notes.rst is added.

@virio-andreyana
Copy link
Copy Markdown
Collaborator Author

The biggest challenge is finding a replacement for the memory_logger function in solve_network and solve_operations_network. The code from vresutils can be seen here https://github.com/FRESNA/vresutils/blob/master/vresutils/benchmark.py

@fneum
Copy link
Copy Markdown
Member

fneum commented May 11, 2023

One could try a minimal memory_logger class that is put into helpers.py:

import os
import time
import threading
import psutil

class MemoryLogger:
    def __init__(self, filename, interval):
        self.stop_event = threading.Event()
        self.thread = threading.Thread(target=self.start, args=(filename, interval))
        self.thread.start()

    def __exit__(self, *args):
        self.stop_event.set()
        self.thread.join()

    def calculate_memory(self, proc):
        total_memory = proc.memory_info().rss
        for child in proc.children(recursive=True):
            total_memory += child.memory_info().rss
        return total_memory

    def start(self, filename, interval):
        with open(filename, 'w') as f:
            while not self.stop_event.is_set():
                process = psutil.Process(os.getpid())
                total_memory = self.calculate_memory(process)
                f.write(f"{time.time()} {total_memory / 10**6}\n")
                time.sleep(interval)

Requires psutil. Can be used:

with MemoryLogger(filename='memory.log', interval=30.0):
    computationally_demanding_function()

@FabianHofmann
Copy link
Copy Markdown
Contributor

FabianHofmann commented May 11, 2023

Or one could use the memory logger from snakemake (via benchmark). But in general, I think it would be good to allow disabling it, as memory logging can slow down processes significantly.

@fneum
Copy link
Copy Markdown
Member

fneum commented May 11, 2023

Agree! The advantage of the vresutils memory logger is that it monitors and logs RAM usage continuously, but honestly I am not sure how important it is...

@virio-andreyana
Copy link
Copy Markdown
Collaborator Author

Looking into snakemake versions of benchmark https://github.com/snakemake/snakemake/blob/main/snakemake/benchmark.py, I believe snakemake has already well documented its resident set size (RSS), virtual memory size and proportional set size (PSS) through its automatic benchmark log. Let's just remove vresutils.benchmark from the code without replacing it.

@fneum
Copy link
Copy Markdown
Member

fneum commented May 11, 2023

I'm happy with this proposal

@virio-andreyana virio-andreyana marked this pull request as ready for review May 11, 2023 15:41
@fneum fneum enabled auto-merge May 12, 2023 05:12
@fneum
Copy link
Copy Markdown
Member

fneum commented May 12, 2023

Looks great! Finally vresutils is out! Just added a few finishing touches.

@fneum fneum merged commit 9012ef0 into master May 12, 2023
@fneum fneum deleted the remove-vresutils branch May 12, 2023 05:53
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.

3 participants