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

Remove vresutils #662

Merged
merged 6 commits into from
May 12, 2023
Merged

Remove vresutils #662

merged 6 commits into from
May 12, 2023

Conversation

virio-andreyana
Copy link
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
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
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
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
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
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
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
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