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

Handle Animate dependency #122

Open
jwallwork23 opened this issue Sep 3, 2024 · 3 comments
Open

Handle Animate dependency #122

jwallwork23 opened this issue Sep 3, 2024 · 3 comments
Assignees
Labels
install Installation instructions/process needs updating

Comments

@jwallwork23
Copy link
Member

Currently Animate is used for computing Hessians.

We should either avoid this dependency or raise useful errors if a user attempts to use this functionality without having installed Animate.

@jwallwork23 jwallwork23 added the install Installation instructions/process needs updating label Sep 3, 2024
@ddundo
Copy link
Member

ddundo commented Sep 17, 2024

Thinking out loud here... Maybe we could have a compute_hessian function in Movement that would first try to import e.g. recover_hessian_clement from animate.recovery (assuming it's a standalone function - which it isn't atm). If that raises ImportError we could try remote fetching and execution. And if that fails we raise an error.

Something like:

import requests
import re

def compute_hessian():
    try:
        from Animate.recovery import recover_hessian_clement
        print("Loaded recover_hessian_clement from Animate.")
        return recover_hessian_clement
    except ImportError:
        print("Animate is not installed. Attempting remote execution...")

    try:
        url = 'https://raw.githubusercontent.com/mesh-adaptation/animate/main/animate/recovery.py'
        
        # Fetch the raw content of the file from GitHub
        response = requests.get(url)
        response.raise_for_status()
        file_content = response.text

        # Extract the 'recover_hessian_clement' function
        compute_hessian_code = re.search(r'def recover_hessian_clement ...).group(0)

        # Execute the extracted function code in the global namespace
        exec(compute_hessian_code, globals())
        print("Loaded recover_hessian_clement via remote execution.")

        return recover_hessian_clement 

    except Exception as e:
        raise ImportError(
            f"Failed to load recover_hessian_clement. Please install Animate or enable remote fetching."
        )

I have a feeling software engineers would hate this, but maybe it's not bad if we really want to avoid Animate dependency for just this one function? :)

@jwallwork23
Copy link
Member Author

Thanks for the suggestion but I don't want to put this kind of thing in the repo. Indeed, it's not good software engineering practice.

What I was thinking of most recently was implementing simple gradient and Hessian recovery methods in Movement with a switch to use the more extensive implementations in Animate if it's been installed.

Alternatively, we could propose to get Clement interpolation merged into Firedrake itself so all the packages, as well as other Firedrake users can make use of it.

I'll put this issue in the "discussion" section for the next meeting.

@jwallwork23
Copy link
Member Author

Could also try to get some gradient and Hessian recovery functionality merged into Firedrake? They might be interested in Clement interpolation, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install Installation instructions/process needs updating
Projects
Development

No branches or pull requests

2 participants