Skip to content

Conversation

@AHsu98
Copy link

@AHsu98 AHsu98 commented Dec 19, 2023

First commit towards adding a group L0 norm. Happy to change the name, perhaps L02 is better. This is defined as the L0 norm of the vector of L2 norms of the indices, weighted by the values of lambda. Still needs a bit of testing I think, but pretty simple, only a couple of lines to change from GroupL2.

Also, in both this and GroupL2, we should call out that we require it to be a separable sum (no indices overlap).

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Hey @AHsu98! Sorry I must have missed this PR. It looks great, thank you! Here are a few minor comments.

Would you mind adding some using tests though? Thanks!

@MohamedLaghdafHABIBOULLAH
Copy link
Collaborator

Thank you @AHsu98, could you please rebase and update this PR ?
It would be nice to add some simple tests and check for allocation using the macro wrappedallocs

AHsu98 and others added 8 commits February 22, 2025 17:04
+ to .+ and some minor syntax changes

Co-authored-by: Dominique <dominique.orban@gmail.com>
…same changes that @dpo suggested on the checking for this groupNormL0 to make groupNormL2 match as well
…in groupNormL0 and groupNormL2 as it was causing tests to error with the way I was checking, and added the groupNormL0 and shiftedGroupNormL0 to the main file. Added groupNormL0 to the tests, and its not erroring, but haven't added the correctness check yet.
@AHsu98
Copy link
Author

AHsu98 commented Feb 23, 2025

Alright, I rebased, made some updates (fixed some original mistakes), and incorporated the previous suggestions. It'd be nice to add a check that the indices are non-overlapping (the naive thing I tried worked for the basic case of vectors of vectors of integers as indices, but caused errors in the tests, maybe from shifts, or multiple indices). I also made the suggested changes to the checks on groupNormL2 as well.
Still remains to add testing and check allocation.

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Hi @AHsu98 ! Sorry for the delay. Just a few cosmetic changes.

Could you please also add some unit tests to valide this code?

ysum = R(0)
for (idx, λ) zip(f.idx, f.lambda)
yt = norm(x[idx])^2
if yt !=0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if yt !=0
if yt > 0

Comment on lines +15 to +24
struct GroupNormL0{R <: Real, RR <: AbstractVector{R}, I}
lambda::RR
idx::I

function GroupNormL0{R, RR, I}(lambda::RR, idx::I) where {R <: Real, RR <: AbstractVector{R}, I}
any(lambda .< 0) && error("weights λ must be nonnegative")
length(lambda) != length(idx) && error("number of weights and groups must be the same")
new{R, RR, I}(lambda, idx)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
struct GroupNormL0{R <: Real, RR <: AbstractVector{R}, I}
lambda::RR
idx::I
function GroupNormL0{R, RR, I}(lambda::RR, idx::I) where {R <: Real, RR <: AbstractVector{R}, I}
any(lambda .< 0) && error("weights λ must be nonnegative")
length(lambda) != length(idx) && error("number of weights and groups must be the same")
new{R, RR, I}(lambda, idx)
end
end
struct GroupNormL0{R <: Real, V <: AbstractVector{R}, I}
lambda::V
idx::I
function GroupNormL0{R, V, I}(lambda::V, idx::I) where {R <: Real, V <: AbstractVector{R}, I}
any(lambda .< 0) && error("weights λ must be nonnegative")
length(lambda) != length(idx) && error("number of weights and groups must be the same")
new{R, V, I}(lambda, idx)
end
end

Comment on lines +40 to +45
function prox!(
y::AbstractArray{R},
f::GroupNormL0{R, RR, I},
x::AbstractArray{R},
γ::R = R(1),
) where {R <: Real, RR <: AbstractVector{R}, I}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function prox!(
y::AbstractArray{R},
f::GroupNormL0{R, RR, I},
x::AbstractArray{R},
γ::R = R(1),
) where {R <: Real, RR <: AbstractVector{R}, I}
function prox!(
y::AbstractArray{R},
f::GroupNormL0{R, V, I},
x::AbstractArray{R},
γ::R = R(1),
) where {R <: Real, V <: AbstractVector{R}, I}

lambda::RR
idx::I

function GroupNormL2{R, RR, I}(lambda::RR, idx::I) where {R <: Real, RR <: AbstractVector{R}, I}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function GroupNormL2{R, RR, I}(lambda::RR, idx::I) where {R <: Real, RR <: AbstractVector{R}, I}
function GroupNormL2{R, V, I}(lambda::V, idx::I) where {R <: Real, V <: AbstractVector{R}, I}

end
any(lambda .< 0) && error("weights λ must be nonnegative")
length(lambda) != length(idx) && error("number of weights and groups must be the same")
new{R, RR, I}(lambda, idx)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
new{R, RR, I}(lambda, idx)
new{R, V, I}(lambda, idx)

Copy link
Member

Choose a reason for hiding this comment

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

There may be others in this file. It’s just easier to remember that V stands for “vector”.


mutable struct ShiftedGroupNormL0{
R <: Real,
RR <: AbstractVector{R},
Copy link
Member

Choose a reason for hiding this comment

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

Use V0, V1, V2, V3 here.

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.

3 participants