Add docstrings to randomstart.py - #20
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves lemke.randomstart usability by documenting existing helper functions and extracting the CLI plotting logic into a library-callable function.
Changes:
- Added docstrings to internal helpers and
main()(to improveclick --helpoutput). - Extracted plotting logic into a new
plot_simplex()helper with a NumPy-style docstring. - Added a
higherdimrange check inplot_simplex()that raisesValueErrorfor invalid values.
Suppressed comments (1)
src/lemke/randomstart.py:110
plot_simplex()is described as a library-callable helper, but it currently prints to stdout and recomputessegmentstartinside the sampling loop. Printing is an unexpected side effect for a library API, and recomputing a constant each iteration is unnecessary. Consider removing the prints and precomputingsegmentstartonce, using it in the loop. Also,fig1is unused; prefix it with_to avoid unused-variable warnings.
if not 3 <= higherdim <= 10:
raise ValueError("higherdim must be between 3 and 10")
print(
f"numpoints={numpoints} accuracy={accuracy} higherdim={higherdim} naiveplot={naiveplot}"
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| plt.scatter([x], [y], s=10000 // accuracy, facecolors="none", | ||
| edgecolors="r") |
There was a problem hiding this comment.
I used 20 instead of 1, since with size 1 the red circles were still too small to actually be visible on the plot.
| def plot_simplex(numpoints=200, accuracy=20, higherdim=3, naiveplot=False): | ||
| """Generate a simplex sampling plot. | ||
|
|
||
| Samples `numpoints` random points from the simplex of dimension `higherdim` | ||
| and projects them onto a 2D triangle (if `higherdim` is greater than 3, | ||
| only the middle 3 components of each point are used, renormalized to sum to 1). | ||
| Plots the raw sampled points in green and their rounded approximations in red. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| numpoints : int | ||
| Number of points to plot. Default is 200. | ||
| accuracy : int | ||
| Denominator x; each coordinate is rounded to the nearest multiple of 1/x. | ||
| Default is 20. Must be between 1 and 10,000,000. | ||
| higherdim : int | ||
| Dimension from which the middle 3 components will be sampled. | ||
| Default is 3. Must be between 3 and 10. | ||
| naiveplot : bool | ||
| Sample naively by normalizing random uniforms (biased toward center). | ||
| Default is False. | ||
|
|
||
| Raises | ||
| ------ | ||
| ValueError | ||
| If `accuracy` or `higherdim` is out of range. | ||
| """ | ||
| if not 3 <= higherdim <= 10: | ||
| raise ValueError("higherdim must be between 3 and 10") |
There was a problem hiding this comment.
after line 36 add:
The probabilities are multiplied by accuracy, then rounded down to their integer parts,
which will be the numerators, augmented by 1 in order of decreasing size of the remainders
(which are less than 1) until they sum to accuracy.
Example: accuracy=10, x = [0.18, .35, .47] becomes [2/10,3/10,5/10].
| Round each entry of an array of probabilities `x` | ||
| to the nearest multiple of 1 / `accuracy`. | ||
| """ | ||
| if not 1 <= accuracy <= MAX_ACCURACY: |
There was a problem hiding this comment.
can we also check that accuracy is an integer? Essential for this to work.
Changes
main()(forclick --helptext).main()into a standaloneplot_simplex()function, which library users can call directly in their code (it will do the same thing as the CLIrandomstartcommand).plot_simplex().higherdiminsideplot_simplex()- invalid values raiseValueError.