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

adding dof manager to params and re-writing params as an equinox module #111

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

Conversation

cmhamel
Copy link
Collaborator

@cmhamel cmhamel commented Mar 3, 2025

This PR does two main things:

  1. Re-writes Params as an equinox module so we can stash the DofManager here. This didn't work out of the box with Params as a namedtuple, hence the change. No runtime affects were encountered.
  2. Keeps backwards comparability in place.

Why did I do this?

With the current setup, if you want to change Dirichlet BCs, you'd need to redefine all energy functions, etc. that implicitly use the DofManager. Now we can just create a new DofManager with the new desired BCs and update this in Params. This will of course likely lead to re-jitting whenever the DofManager changes which should at most be a handful of times even in the most complex analyses.

Copy link

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 80.76923% with 10 lines in your changes missing coverage. Please review.

Project coverage is 77.25%. Comparing base (fc47de4) to head (3247e1c).

Files with missing lines Patch % Lines
optimism/Objective.py 80.76% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   77.26%   77.25%   -0.02%     
==========================================
  Files          62       62              
  Lines        5230     5267      +37     
==========================================
+ Hits         4041     4069      +28     
- Misses       1189     1198       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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