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

Regularisation Interface #16

Open
casv2 opened this issue Jul 23, 2020 · 6 comments
Open

Regularisation Interface #16

casv2 opened this issue Jul 23, 2020 · 6 comments

Comments

@casv2
Copy link
Collaborator

casv2 commented Jul 23, 2020

To keep the agnostic nature we need to start adding regularisation specific functions to aPIPs/ACE separately. These will then return a Γ matrix, with which we solve the regularised LSQ problem here in IPFitting.

It seems this is really only an issue for Laplacian regularisation, right? For ridge regression the Γ matrix is just the identity (times some factor) and therefore independent of the basis.

More practically speaking should I look at adding this Γ function to ACE/src/regularisers.jl first? We can then merge later if we agree through a PR?

@cortner @gabor1

@cortner
Copy link
Member

cortner commented Jan 24, 2022

ah - perfect place to discuss this.

@cortner
Copy link
Member

cortner commented Jan 24, 2022

I've been mulling this over .... and I now think that what you already started (@casv2 ) is correct; for now, a regularizer should simply be an AbstractMatrix. Depending on what kind of matrix it is will determine which solvers are available. For example, if it is <: Diagonal, then we can do the rescaling instead of tychonov and that means we can do LSQR or RRQR. But if it is a general matrix than fewer things are possible. Do you agree that we can simply put that kind of check into the solvers?

And then we put into ACE1.jl exactly the one-liner you had in the Slack to produce a simple regularizer which is diagonal. @casv2 I'd be grateful if you can make a PR to ACE1.jl to set this up in a way that is convenient for you and will work as expected with IPFitting.jl

@casv2
Copy link
Collaborator Author

casv2 commented Jan 25, 2022

Ok, let me add a PR to ACE1.jl putting the oneliner in there such that we set up the P matrix while fitting and feed it into the latest IPFitting. I think that makes sense. But we can apply the P in any case right? I've heard people want to try using the Laplacian rescaling on other solvers too? Not sure we need the check, this gives full flexibility

@cortner
Copy link
Member

cortner commented Jan 25, 2022

I don't see why it shouldn't - we may have to modify the solvers though.

@cortner
Copy link
Member

cortner commented Jan 25, 2022

We should also document this "interface" somewhere. Maybe the ACE1docs for now are ok, but eventually for ACE2 and ACEfit we need better "developer docs" that clearly specify those interfaces.

@cortner
Copy link
Member

cortner commented Jan 28, 2022

once this is in ACE1 and there is documentation of this discussion, this can be closed

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

No branches or pull requests

2 participants