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

Refactor SchurComplementW for ClimaTimesteppers #1364

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Conversation

charleskawczynski
Copy link
Member

@charleskawczynski charleskawczynski commented Jul 11, 2023

This PR is a peel off from #1358.

This PR adds temp1 and temp2 to SchurComplementW, and defines

linsolve!(::Type{Val{:init}}, f, u0; kwargs...) = _linsolve!
_linsolve!(x, A, b, update_matrix = false; kwargs...) =
    LinearAlgebra.ldiv!(x, A, b)

# Function required by Krylov.jl (x and b can be AbstractVectors)
# See https://github.com/JuliaSmoothOptimizers/Krylov.jl/issues/605 for a
# related issue that requires the same workaround.
function LinearAlgebra.ldiv!(x, A::SchurComplementW, b)
    A.temp1 .= b
    LinearAlgebra.ldiv!(A.temp2, A, A.temp1)
    x .= A.temp2
end

and the original _linsolve contents are in

function LinearAlgebra.ldiv!(
    x::Fields.FieldVector,
    A::SchurComplementW,
    b::Fields.FieldVector,
)

(the pattern used in ClimaAtmos). It also renames _linsolve! to test_linsolve! to avoid a name collision in the test suite.

It's a much smaller PR than it appears, due to indenting.

@charleskawczynski
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

Copy link
Member

@juliasloan25 juliasloan25 left a comment

Choose a reason for hiding this comment

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

LGTM!

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 14, 2023
1364: Refactor SchurComplementW for ClimaTimesteppers r=charleskawczynski a=charleskawczynski

This PR is a peel off from #1358.

This PR adds `temp1` and `temp2` to `SchurComplementW`, and defines
```julia
linsolve!(::Type{Val{:init}}, f, u0; kwargs...) = _linsolve!
_linsolve!(x, A, b, update_matrix = false; kwargs...) =
    LinearAlgebra.ldiv!(x, A, b)

# Function required by Krylov.jl (x and b can be AbstractVectors)
# See JuliaSmoothOptimizers/Krylov.jl#605 for a
# related issue that requires the same workaround.
function LinearAlgebra.ldiv!(x, A::SchurComplementW, b)
    A.temp1 .= b
    LinearAlgebra.ldiv!(A.temp2, A, A.temp1)
    x .= A.temp2
end
```
and the original `_linsolve` contents are in
```julia
function LinearAlgebra.ldiv!(
    x::Fields.FieldVector,
    A::SchurComplementW,
    b::Fields.FieldVector,
)
```

(the pattern used in ClimaAtmos). It also renames `_linsolve!` to `test_linsolve!` to avoid a name collision in the test suite.

It's a much smaller PR than it appears, due to indenting.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 14, 2023

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 14, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit dc43686 into main Jul 14, 2023
7 checks passed
@bors bors bot deleted the ck/SchurComplementW branch July 14, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants