fix: remove problematic defaults from `sample_last_cit()` parameters
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
agebinsandprefparameters insample_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:
-
agebinsmust be ≥ 1 and controls aging discretization -
prefshould be a vector of lengthagebins + 1 - Power-law decay
(1:(agebins + 1))^-3is 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_citnot well documented</issue_title> <issue_description>Unclear howagebins = n / 7100, pref = (1:(agebins + 1))^-3are 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
agebinsparameter;- 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>
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>@schochastics
- Fixes igraph/rigraph#1676
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
@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.