-
Notifications
You must be signed in to change notification settings - Fork 9
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
Labels
enhancement
New feature or request
Comments
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ClimaAtmos currently uses
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).
The text was updated successfully, but these errors were encountered: