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

Replace AbstractMatrixCFcn for new type LinearMap #85

Closed
wants to merge 6 commits into from
Closed

Replace AbstractMatrixCFcn for new type LinearMap #85

wants to merge 6 commits into from

Conversation

lopezm94
Copy link
Contributor

@lopezm94 lopezm94 commented Aug 18, 2016

Replace AbstractMatrixCFcn and children for a new unique type LinearMap. LinearMap can be created without any functions and defaults them to identity, multiplication and ctranspose multiplication can be added individually. It also switches the functions when it is ctransposed to know at every moment which function to use.

Simpler to use and there is only one type for the user to handle.

@andreasnoack
Copy link
Member

Maybe you are aware but remember that functions have types in 0.5 so it might not be necessary to wrap them in a type. E.g. you can define

Base.eltype(::typeof(f)) = Float64
Base.size(::typeof(f)) = (10,10)
Base.*(::typeof(f), x) = f(x)

I'm not sure if it fits all your needs here but I could get a long way with this recently when I wrote an iterative algorithm.

@lopezm94
Copy link
Contributor Author

I think it won't help in this case, the type wraps two functions: multiplication and ctranspose multiplication. When ctransposing it is desirable to have the wrapper to organize which function to use.

@codecov-io
Copy link

codecov-io commented Aug 22, 2016

Codecov Report

Merging #85 into master will decrease coverage by 1.52%.
The diff coverage is 44%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #85      +/-   ##
=========================================
- Coverage   87.73%   86.2%   -1.53%     
=========================================
  Files          18      17       -1     
  Lines        1223    1124      -99     
=========================================
- Hits         1073     969     -104     
- Misses        150     155       +5
Impacted Files Coverage Δ
src/rlinalg.jl 83.33% <0%> (ø) ⬆️
src/common.jl 61.81% <45.83%> (-13.19%) ⬇️
src/svdl.jl 89.2% <0%> (-3.76%) ⬇️
src/gmres.jl 96.66% <0%> (-0.21%) ⬇️
src/lsqr.jl 99.11% <0%> (-0.04%) ⬇️
src/stationary.jl 100% <0%> (ø) ⬆️
src/chebyshev.jl 100% <0%> (ø) ⬆️
src/cg.jl 100% <0%> (ø) ⬆️
src/lanczos.jl 100% <0%> (ø) ⬆️
src/lsmr.jl 100% <0%> (ø) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5cc6d5...757d327. Read the comment docs.

@jiahao jiahao mentioned this pull request Aug 22, 2016
@jiahao
Copy link
Contributor

jiahao commented Aug 23, 2016

In the code it would be worth adding comments as to what the A and B type parameters mean.

@lopezm94 lopezm94 changed the title Remake AbstractMatrixCFcn Replace AbstractMatrixCFcn for new type LinearMap Aug 23, 2016
@jiahao
Copy link
Contributor

jiahao commented Aug 25, 2016

@lopezm94 has tried the function type approach in this branch: https://github.com/lopezm94/IterativeSolvers.jl/blob/FunctionDispatch/src/common.jl#L79-L117

Basically the main problems are that of scope and delaying the evaluation of the definitions.

@andreasnoack any thoughts?

@andreasnoack
Copy link
Member

Can either of you explain the "delaying" part to me?

Couldn't it be e.g. Base.eltype(::typeof(mu)) = typ instead or am I missing something?

@lopezm94
Copy link
Contributor Author

lopezm94 commented Aug 27, 2016

Reason:

julia> function f(g::Function)
           k(::typeof(g)) = nothing
       end
ERROR: UndefVarError: g not defined

Julia does something to ::typeof at compile time, so there is a need to delay the evaluation or it will go out of scope

julia> function f(g::Function)
           @eval k(::typeof($g)) = nothing
       end
f (generic function with 1 method)

julia> f(identity)
k (generic function with 1 method)

@lopezm94
Copy link
Contributor Author

And then there's the problem with the scope, if the user deletes the object the functions might be left behind.

@lopezm94
Copy link
Contributor Author

lopezm94 commented May 8, 2017

Closing in favor of #104

@lopezm94 lopezm94 closed this May 8, 2017
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.

4 participants