Skip to content

Loosen type-constraints don't enforce <:AbstractVector - #1033

Draft
vchuravy wants to merge 1 commit into
JuliaSmoothOptimizers:mainfrom
vchuravy:vc/loosen_constraints
Draft

vchuravy wants to merge 1 commit into
JuliaSmoothOptimizers:mainfrom
vchuravy:vc/loosen_constraints

Conversation

@vchuravy

Copy link
Copy Markdown

@lcw and I just run into a case where we have a data-structure GridArray that for the purpose of Krylov.jl acts like a Vector and we have defined the Krylov custom workspace API for,
but for other reasons presents as AbstractArray{Float64, 2}.

IIRC the last time we spoke about this, there was an argument that the reason why Krylov.jl has this type constraint is so that the block-gmres API can use Matrix as an input.

@amontoison

Copy link
Copy Markdown
Member

@vchuravy Is it not easier to do a "wrapper" that is an AbstractVector like I did in the documentation?
https://jso.dev/Krylov.jl/dev/custom_workspaces/#Definition-and-usage-of-the-HaloVector

I am scared to open a can of worms by relaxing the type.

@codecov

codecov Bot commented Oct 28, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.64%. Comparing base (9536ef7) to head (1b84427).
⚠️ Report is 96 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
+ Coverage   94.68%   97.64%   +2.96%     
==========================================
  Files          45       49       +4     
  Lines        8027     9463    +1436     
==========================================
+ Hits         7600     9240    +1640     
+ Misses        427      223     -204     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vchuravy

Copy link
Copy Markdown
Author

Perhaps, but @lcw GridArray is a wrapper type... So it felt a bit silly to add a second wrapper type to present a vector to Krylov.jl

@amontoison

Copy link
Copy Markdown
Member

I can understand that but can you show me what is doing GridArray such that I better understand the type?
I think I have an alternative solution that you will like 🙂

Comment thread src/gmres.jl
:(b::AbstractArray{FC}))

def_optargs_gmres = (:(x0::AbstractVector),)
def_optargs_gmres = (:(x0::AbstractArray{FC}),)

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.

We don't to force FC because sometimes we warm-start from a solution in lower precision.
We could use a direct solver in Float32 to compute a very good approximate and then get more digits with a Krylov solvers.

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.

2 participants