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

Fixing errors related to default L2 grad in adjoints #3579

Merged
merged 16 commits into from
Jun 5, 2024

Conversation

Ig-dolci
Copy link
Contributor

@Ig-dolci Ig-dolci commented May 21, 2024

Description

This PR seeks the application of Riesz maps using the L2 norm as the default in Firedrake for adjoint gradients. Implementing this change requires some coding modifications.

Some pyadjoint tests will fail because they were designed with Riesz maps using the l2 norm. To address these test failures, refer to Pyadjoint PR 149, which has the necessary fixes.

@Ig-dolci Ig-dolci marked this pull request as ready for review May 23, 2024 13:08
@Ig-dolci Ig-dolci requested review from APaganini and dham May 23, 2024 13:09
@Ig-dolci Ig-dolci linked an issue May 23, 2024 that may be closed by this pull request
Copy link
Contributor

@APaganini APaganini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

David should also look at adjoint_utils/function.py and at adjoint_utils/blocks/function.py

tests/regression/test_adjoint_operators.py Outdated Show resolved Hide resolved
tests/regression/test_adjoint_operators.py Outdated Show resolved Hide resolved
tests/regression/test_adjoint_operators.py Outdated Show resolved Hide resolved
tests/regression/test_adjoint_operators.py Show resolved Hide resolved
tests/regression/test_adjoint_operators.py Show resolved Hide resolved
tests/regression/test_adjoint_operators.py Show resolved Hide resolved
firedrake/adjoint_utils/blocks/function.py Outdated Show resolved Hide resolved
firedrake/adjoint_utils/blocks/function.py Outdated Show resolved Hide resolved
firedrake/adjoint_utils/function.py Outdated Show resolved Hide resolved
firedrake/adjoint_utils/function.py Outdated Show resolved Hide resolved
@Ig-dolci Ig-dolci marked this pull request as draft May 29, 2024 13:04
@Ig-dolci Ig-dolci marked this pull request as ready for review June 3, 2024 09:43
@dham dham enabled auto-merge (squash) June 5, 2024 15:36
@Ig-dolci Ig-dolci dismissed APaganini’s stale review June 5, 2024 19:30

Stealing Alberto's approval.

@dham dham merged commit 9153e39 into master Jun 5, 2024
10 checks passed
@dham dham deleted the dolci/adapting_to_L2_riesmaps_adj_grad branch June 5, 2024 19:30
Ig-dolci added a commit that referenced this pull request Jul 6, 2024
Also add test to verify riesz maps for grad adjoints
---------

Co-authored-by: Alberto Paganini <[email protected]>
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.

BUG: Riesz representation default options is not L2 but l2
4 participants