rigraph icon indicating copy to clipboard operation
rigraph copied to clipboard

fix: remove problematic defaults from `sample_last_cit()` parameters

Open Copilot opened this issue 4 months ago • 1 comments

The sample_last_cit() function had defaults agebins = n / 7100 and pref = (1:(agebins + 1))^-3 that only work for n ≥ 7100, causing silent failures with confusing error messages for smaller graphs.

Changes

  • Removed defaults for agebins and pref parameters in sample_last_cit() only
  • Preserved deprecated function: The deprecated lastcit.game() function retains its original default parameters to maintain backward compatibility (per project guideline: "Never change deprecated functions")
  • Enhanced parameter documentation explaining:
    • agebins must be ≥ 1 and controls aging discretization
    • pref should be a vector of length agebins + 1
    • Power-law decay (1:(agebins + 1))^-3 is a typical choice
  • Added examples demonstrating proper usage
  • Added tests for parameter validation and various configurations

Usage

The new function now requires explicit parameters:

# t^-3 power-law decay with 5 age bins
g <- sample_last_cit(100, edges = 1, agebins = 5, pref = (1:6)^-3)

The deprecated function maintains backward compatibility:

# Deprecated function still works with defaults for n ≥ 7100
g_old <- lastcit.game(7100)  # Still works

Fixes #1776

Original prompt

This section details on the original issue you should resolve

<issue_title>Default parameters of sample_last_cit not well documented</issue_title> <issue_description>Unclear how agebins = n / 7100, pref = (1:(agebins + 1))^-3 are determined The function only works if agebins>1, so by default it needs 7100 nodes. </issue_description>

Comments on the Issue (you are @copilot in this section)

@szhorvat This is a legacy function, probably motivated by Gábor's thesis work. I don't know how much it is used in practice ...

I wouldn't put too much effort into it.

IMO reasonable options are:

  • Add one to the agebins parameter;
  • Or just remove defaults entirely, and suggest the $t^{-3}$ decay function in the documentation (phrased as an example).

Any opinion @ntamas ? I see this one is not even exposed in Python.</comment_new> <comment_new>@schochastics

Just stumbled upon it when cleaning games.R and its tests. I think your suggestions are reasonable but I will have at least a short look into the literature cause I am also not familiar with this</comment_new>
  • Fixes igraph/rigraph#1676

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot avatar Oct 26 '25 21:10 Copilot

@copilot: Merge main, resolve conflicts as needed, and obey updated instructions.

Merged main in eb0808d. Also reverted changes to the deprecated lastcit.game() function per the updated "Never change deprecated functions" instruction in b6ed4d3. The new sample_last_cit() now requires explicit agebins and pref parameters while the deprecated function maintains backward compatibility with its original defaults.

Copilot avatar Oct 26 '25 23:10 Copilot