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

Define indefinite column integrals for arbitrary functions #1492

Closed
charleskawczynski opened this issue Oct 9, 2023 · 2 comments · Fixed by #1504
Closed

Define indefinite column integrals for arbitrary functions #1492

charleskawczynski opened this issue Oct 9, 2023 · 2 comments · Fixed by #1504
Labels
enhancement New feature or request

Comments

@charleskawczynski
Copy link
Member

charleskawczynski commented Oct 9, 2023

ClimaAtmos currently uses

    prob = SciMLBase.ODEProblem(dp_dz, p_0, (FT(0), z_max))
    return SciMLBase.solve(
        prob,
        ODE.Tsit5(),
        reltol = 10eps(FT),
        abstol = 10eps(FT),
    )

to compute a vertical indefinite integral. We currently only need this for initialization, and CliMA/ClimaAtmos.jl#2183 implements this in a way that seems to work (MSE table changes are small). We could start by porting that over to ClimaCore and adding unit tests. Next (if we need) we could make this work on the GPU, which I think shouldn't be too difficult (it may work already as I've followed a similar pattern used for the other indefinite integral on fields).

@charleskawczynski charleskawczynski added the enhancement New feature or request label Oct 9, 2023
@simonbyrne
Copy link
Member

You added something in #1380: is there anything else we need?

@charleskawczynski
Copy link
Member Author

charleskawczynski commented Oct 10, 2023

You added something in #1380: is there anything else we need?

Here's a summary:

Both indefinite integrals need unit tests, it looks like @dennisYatunin pushed since the last bors try, so I just bors try'd again to see if he fixed the unit tests there. Maybe we can re-purpose them for the 3rd bullet implementation.

bors bot added a commit that referenced this issue Oct 20, 2023
1504: Add and test function-based indefinite integrals r=charleskawczynski a=charleskawczynski

This PR adds an indefinite integral implementation that uses a function-based input argument. Concretely, given `ϕ(z)`, this interface allows us to solve for `∂ϕ/∂z = f(ϕ,z)`. The existing field-based implementation cannot be used in this situation, because `f` is a function of `ϕ`, which is what we're computing. So, this implementation uses RootSolvers to find `ϕ` at the next level in order to provide a somewhat generic interface.

Unfortunately, one limitation of this is that `ϕ` must be a scalar field, and it cannot be, e.g., a `NamedTuple`. We could try to lift this assumption, but that would require adjustments to RootSolvers.

Closes #1492.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit that referenced this issue Oct 23, 2023
1504: Add and test function-based indefinite integrals r=charleskawczynski a=charleskawczynski

This PR adds an indefinite integral implementation that uses a function-based input argument. Concretely, given `ϕ(z)`, this interface allows us to solve for `∂ϕ/∂z = f(ϕ,z)`. The existing field-based implementation cannot be used in this situation, because `f` is a function of `ϕ`, which is what we're computing. So, this implementation uses RootSolvers to find `ϕ` at the next level in order to provide a somewhat generic interface.

Unfortunately, one limitation of this is that `ϕ` must be a scalar field, and it cannot be, e.g., a `NamedTuple`. We could try to lift this assumption, but that would require adjustments to RootSolvers.

Closes #1492.

1515: remove distributed TempestRemap tests r=charleskawczynski a=simonbyrne

This isn't how we should do distributed remapping, so i'll just disable them.

Avoids #1513.

- [x] Code follows the [style guidelines](https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/) OR N/A.
- [x] Unit tests are included OR N/A.
- [x] Code is exercised in an integration test OR N/A.
- [x] Documentation has been added/updated OR N/A.


Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
Co-authored-by: Simon Byrne <simonbyrne@gmail.com>
@bors bors bot closed this as completed in 1e36b66 Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants