Skip to content

Rename low-rank Choleksy factor (LRCF) to low-rank (LR) - #2574

Open
ullmannsven wants to merge 7 commits into
mainfrom
renaming_ofLRCF_to_LR
Open

Rename low-rank Choleksy factor (LRCF) to low-rank (LR) #2574
ullmannsven wants to merge 7 commits into
mainfrom
renaming_ofLRCF_to_LR

Conversation

@ullmannsven

Copy link
Copy Markdown
Contributor

Renaming LRCF to LR across all relevant places within pymor. The alternative would be to rename LRADI to LRCFADI, which is currently not the preferred option.

dummy PR in order to have it open before the soft freeze. I will work on this as soon as #2572 is merged.

@ullmannsven ullmannsven added this to the 2026.1 milestone Jul 16, 2026
@ullmannsven ullmannsven self-assigned this Jul 16, 2026
@ullmannsven ullmannsven added the pr:change Change in existing functionality label Jul 16, 2026
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.25000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.08%. Comparing base (67665e6) to head (1a4be65).

Files with missing lines Patch % Lines
src/pymor/solvers/matrix_equations/default.py 61.53% 10 Missing ⚠️
src/pymor/models/iosys.py 93.02% 3 Missing ⚠️
src/pymor/solvers/matrix_equations/lradi.py 75.00% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
github_actions 78.31% <91.25%> (-0.01%) ⬇️
gitlab_ci 88.00% <91.25%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/pymor/bindings/scipy.py 89.41% <100.00%> (ø)
src/pymor/bindings/slycot.py 96.46% <100.00%> (ø)
src/pymor/reductors/bt.py 86.84% <100.00%> (ø)
src/pymor/reductors/sobt.py 83.42% <100.00%> (ø)
src/pymor/solvers/matrix_equations/equations.py 94.32% <100.00%> (ø)
src/pymor/solvers/matrix_equations/interface.py 86.27% <100.00%> (ø)
src/pymor/solvers/matrix_equations/lrradi.py 86.74% <100.00%> (-0.16%) ⬇️
src/pymordemos/heat.py 97.16% <100.00%> (ø)
src/pymordemos/parametric_delay.py 96.00% <ø> (ø)
src/pymordemos/parametric_heat.py 99.50% <ø> (ø)
... and 10 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ullmannsven
ullmannsven force-pushed the renaming_ofLRCF_to_LR branch from 468b897 to c858323 Compare August 9, 2026 12:50

@pmli pmli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please also change appearances of "low-rank Cholesky factor" to "low-rank factor".

@ullmannsven

Copy link
Copy Markdown
Contributor Author

Please also change appearances of "low-rank Cholesky factor" to "low-rank factor".

okay, I thought to leave this as it is in order to have additional descriptive value, but tbh i dont have an opinion on this and if its clear anyway that this is a Choleksy factor then i will remove it. Thanks for taking a look so fast!

@pmli

pmli commented Aug 9, 2026

Copy link
Copy Markdown
Member

Please also change appearances of "low-rank Cholesky factor" to "low-rank factor".

okay, I thought to leave this as it is in order to have additional descriptive value, but tbh i dont have an opinion on this and if its clear anyway that this is a Choleksy factor then i will remove it. Thanks for taking a look so fast!

Well, that's the whole issue with "LRCF" to begin with, because $Z Z^T$ is not a Cholesky factorization unless $Z$ is a square lower-triangular matrix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:change Change in existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants