Skip to content

Conversation

@amal-ghamdi
Copy link
Contributor

@amal-ghamdi amal-ghamdi commented Feb 11, 2025

closes #620

@amal-ghamdi amal-ghamdi requested review from chaozg and jakobsj October 8, 2025 10:04
Copy link
Contributor

@chaozg chaozg left a comment

Choose a reason for hiding this comment

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

Hi @amal-ghamdi , thanks for implementing this feature.

Do you plan to have more demos on clarifying how this observation function could be used? For example, I wonder with an observation_map like lambda u, grid_obs: u**2, how do we compute the jacobian? I mean could the gradient be computed automatically? Is the chain product broken or 'preserved'? I guess a more common observation would be lambda u, grid:u[some indices]. Can we also compute the gradient in this case and use ULA/NUTS?

@amal-ghamdi
Copy link
Contributor Author

Hi @amal-ghamdi , thanks for implementing this feature.

Do you plan to have more demos on clarifying how this observation function could be used? For example, I wonder with an observation_map like lambda u, grid_obs: u**2, how do we compute the jacobian? I mean could the gradient be computed automatically? Is the chain product broken or 'preserved'? I guess a more common observation would be lambda u, grid:u[some indices]. Can we also compute the gradient in this case and use ULA/NUTS?

Many thanks @chaozg for your review. I updated a howto (demos/howtos/TimeDependentLinearPDE.py) we already have to show some usage of the observation_map. The gradient is not supported automatically, the gradient of the entire forward map can be provided by the user when creating the PDEModel object. More elegant or efficient features can be done (like providing gradient for observation operator), but, I believe, it is not within the scope of this PR.

On the posterior level, one can enable FD gradient, regardless of the underlying model.

@amal-ghamdi
Copy link
Contributor Author

Many thanks @chaozg and @jakobsj for your valuable comments! I updated the PR quite a bit to address them. I am re-requesting your review.

@amal-ghamdi amal-ghamdi requested review from chaozg and jakobsj October 27, 2025 16:44
Copy link
Contributor

@chaozg chaozg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jakobsj jakobsj left a comment

Choose a reason for hiding this comment

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

Changes look good and to comprehensively address the comments received. Only some very minor in line comments, so approving now.

amal-ghamdi and others added 2 commits November 6, 2025 09:26
Co-authored-by: Jakob Sauer Jørgensen <[email protected]>
Co-authored-by: Jakob Sauer Jørgensen <[email protected]>
@amal-ghamdi amal-ghamdi merged commit e8fd90f into main Nov 6, 2025
5 checks passed
@amal-ghamdi amal-ghamdi deleted the access_pde_sol branch November 6, 2025 06:26
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.

Enable general observation function in PDE and accessing original solution

4 participants