Add LightGCN support to FlexMF

[mdekstrand]

This updates FlexMF to have LightGCN support (for implicit feedback) that is activated with a new convolution_layers configuration setting. Zero layers gets classical matrix factorization.

Closes #1019.

[codecov]

Codecov Report

❌ Patch coverage is 88.77551% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.48%. Comparing base (82d88b3) to head (3bcfe0e).
⚠️ Report is 24 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lenskit/flexmf/_training.py	70.83%	7 Missing ⚠️
src/lenskit/flexmf/_model.py	93.02%	3 Missing ⚠️
src/lenskit/training.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
+ Coverage   89.47%   89.48%   +0.01%     
==========================================
  Files         222      222              
  Lines       15372    15441      +69     
==========================================
+ Hits        13754    13818      +64     
- Misses       1618     1623       +5     

☔ 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.

[samiravaez]

user and item bias are getting default values in different places and I'm not sure which one is used at the end but the paper doesn't use bias terms for scoring as I saw, so I think it makes sense for both defaults to be false

[mdekstrand]

@samiravaez

user and item bias are getting default values in different places and I'm not sure which one is used at the end but the paper doesn't use bias terms for scoring as I saw, so I think it makes sense for both defaults to be false

This is a good thing to think about… there is a tradeoff in the configurability and defaults:

• should the defaults be optimized for "pretty good" or for fidelity to the original paper? (I lean towards "pretty good".)
• should the defaults be consistent across configurations, or have complex dependencies on other settings? (I lean towards consistent, to make it easier to configure the model and to make it easier to document.)

The original BPR doesn't include a bias term, and I don't remember if the logistic matrix factorization paper did or not.

We should experiment with this some to confirm, but I expect including item biases will usually be better, because the model doesn't have to try to model overall item popularity with the embeddings — the embeddings can focus on modeling user preference after popularity is accounted for. User biases only make sense with the logistic loss function.

I agree that we shouldn't have differing defaults. I pushed a change that removes defaulting entirely from the model class, and the explicit scorer now explicitly passes True.

(At some point, we should probably support convolution layers with explicit feedback too.)

[samiravaez]

@samiravaez

user and item bias are getting default values in different places and I'm not sure which one is used at the end but the paper doesn't use bias terms for scoring as I saw, so I think it makes sense for both defaults to be false

This is a good thing to think about… there is a tradeoff in the configurability and defaults:

• should the defaults be optimized for "pretty good" or for fidelity to the original paper? (I lean towards "pretty good".)
• should the defaults be consistent across configurations, or have complex dependencies on other settings? (I lean towards consistent, to make it easier to configure the model and to make it easier to document.)

The original BPR doesn't include a bias term, and I don't remember if the logistic matrix factorization paper did or not.

We should experiment with this some to confirm, but I expect including item biases will usually be better, because the model doesn't have to try to model overall item popularity with the embeddings — the embeddings can focus on modeling user preference after popularity is accounted for. User biases only make sense with the logistic loss function.

I agree that we shouldn't have differing defaults. I pushed a change that removes defaulting entirely from the model class, and the explicit scorer now explicitly passes True.

(At some point, we should probably support convolution layers with explicit feedback too.)

I agree on the consistency issue, that makes sense.
But having a different default for each version of FlexMF also makes sense to me. I usually lean toward setting defaults to match the originally proposed method. When people use it, the name kind of suggests that it matches the original paper. And since keeping the model minimal is kind of the core idea of the paper, it makes sense to avoid training extra parameters.