Add GlobalSortAggr to nn.aggr package.

[Padarn]

Add GlobalSortAggr to the new nn.aggr packages replace existing global_sort_pool from nn.glob.sort.

Addresses item in #4712

WIP items to address:

• input dim_size is not checked or used
• input dim is not yet respected # skipping as to_dense_batch does not support this yet
• depreciate nn.glob version (replace it in existing implementations?)

[rusty1s]

Thank you! Looks amazing. If possible, let's deprecate the corresponding function in torch_geometric.nn.glob in this PR as well.

[Padarn]

Makes sense. Will add it to the list. Do you mean to apply a depreciated tag or to remove it from the rest of the codebase? (maybe these two can be done separately?)

[Padarn]

Not sure why I'm getting failures in the glob tests - do we want to keep them?

[Padarn]

cannot use the new class here as its not a replacement (new class doesn't take k in the function call)

[Padarn]

we could make it a kwarg if we want it to work exactly the same way

[rusty1s]

I think you can just define it here:

@deprecated(...)
def global_sort_pool(...):
    module = GlobalSortAggr(...)
    return module(...)

[Padarn]

yeah okay this is a better solution, thanks!

[rusty1s]

I think the issue with failing tests is that there exists multiple test_sort.py files (which is not allowed by pytest). Try removing glob/aggr/test_sort.py.

[rusty1s]

I think you can just define it here:

@deprecated(...)
def global_sort_pool(...):
    module = GlobalSortAggr(...)
    return module(...)

[codecov]

Codecov Report

Merging #4957 (3cd30b6) into master (c6675c4) will increase coverage by 0.00%.
The diff coverage is 94.11%.

@@           Coverage Diff           @@
##           master    #4957   +/-   ##
=======================================
  Coverage   82.79%   82.79%           
=======================================
  Files         330      330           
  Lines       17991    18004   +13     
=======================================
+ Hits        14896    14907   +11     
- Misses       3095     3097    +2     

Impacted Files	Coverage Δ
torch_geometric/nn/glob/__init__.py	83.33% <60.00%> (-9.53%)	⬇️
torch_geometric/nn/aggr/__init__.py	100.00% <100.00%> (ø)
torch_geometric/nn/aggr/base.py	93.61% <100.00%> (ø)
torch_geometric/nn/aggr/sort.py	100.00% <100.00%> (ø)

---

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 c6675c4...3cd30b6. Read the comment docs.

[Padarn]

thanks, I'd not have found that without a lot of pain 😓 (sorry about the force push, had a local conflict that was giving me a lot of trouble)

[Padarn]

Looks okay now. May as well remove the old version at this point. WDYT?

[rusty1s]

Yes, please remove it :)

[Padarn]

All done :-)

[lightaime]

Thanks, @Padarn. LGTM!

[lightaime]

nit-pick comment: I think it may be better to rename SortAggr as SortAggregation to be consistent with other Aggregation classes. WDYT?

[Padarn]

I agree @lightaime! So: #5085