Skip to content

First batch of doc <-> header <-> source alignment#2604

Merged
fredrik-johansson merged 4 commits intoflintlib:mainfrom
edgarcosta:fix-param-names-batch1
Mar 16, 2026
Merged

First batch of doc <-> header <-> source alignment#2604
fredrik-johansson merged 4 commits intoflintlib:mainfrom
edgarcosta:fix-param-names-batch1

Conversation

@edgarcosta
Copy link
Copy Markdown
Member

@edgarcosta edgarcosta commented Mar 11, 2026

First batch of parameter name consistency fixes, applying the auto-fix tool from #2602 to arb_calc, padic_poly, and ca_vec, plus manual fixes and swap corrections found in other modules during review.

The fixes are split into three commits to make review easier:

Commit 1: Manual fixes

Cases that need human judgement rather than automated renaming:

Module Function Change Reason
thread_pool thread_pool_init l -> size in header Header had terse name; source and doc both used size
padic_poly _padic_poly_fprint_pretty swapped val, len -> len, val in header and doc Header and doc had (val, len) but source had (len, val). Also fixed the _padic_poly_print_pretty wrapper which was forwarding in the wrong order
padic_poly padic_poly_set_coeff_padic f -> poly in header and doc Source used poly; header and doc both had f
ca_vec ca_vec_set_length res -> vec in header Source and doc both used vec
fmpz_extras fmpz_lshift_mpn d, dn, sgnbit -> src, n, negative in header and source Doc had more descriptive names

Commit 2: Auto-fix parameter names

Automated renames in arb_calc, padic_poly, and ca_vec where the source used different names from the header. Header names are treated as canonical.

Notable cases:

  • padic_poly_set_coeff_padic: renaming parameter x -> c in the source required also renaming a local variable int c -> int alloc to avoid a name collision
  • arb_calc_isolate_roots: renamed blocks -> found and block -> interval in source to match header

Commit 3: Fix swapped parameter names

Cross-module swap fixes found by the checker's swap detection:

Module Functions Change Details
arb arb_set, arb_set_si, arb_set_ui, arb_set_d, arb_set_fmpz, arb_neg_round swapped x <-> y in source The rest of arb uses y = output, x = input; these had it backwards
gr_poly _gr_poly_div_series_newton, _gr_poly_div_series_invmul swapped A <-> B in source and doc Convention is (res, A, Alen, B, Blen) where A = dividend, B = divisor; source and doc had them flipped
nmod_mat nmod_mat_neg swapped A <-> B in doc Doc had (A, B) but header and source both had (B, A)

@albinahlback
Copy link
Copy Markdown
Collaborator

Hmm, I think Edgar is alive after all


void
ca_vec_printn(const ca_vec_t vec, slong digits, ca_ctx_t ctx)
ca_vec_printn(const ca_vec_t poly, slong digits, ca_ctx_t ctx)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vec seems more appropriate than poly here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest commit this still seems to be poly?

@edgarcosta edgarcosta force-pushed the fix-param-names-batch1 branch from 8e88593 to 4397734 Compare March 11, 2026 22:21
@edgarcosta edgarcosta mentioned this pull request Mar 11, 2026
1 task
Cases where the automated tool cannot safely rename, or where the
header was the one that needed fixing:

- thread_pool_init: header had 'l', source and doc use 'size'
- _padic_poly_fprint_pretty: header had val/len swapped relative
  to the source definition (len, val). Also fix the wrapper
  _padic_poly_print_pretty to match.
- ca_vec_set_length: header had 'res', source and doc agree on 'vec'
- fmpz_lshift_mpn: header and source used terse names (d, dn,
  sgnbit), doc used more descriptive names (src, n, negative).
  Updated header and source to match the doc.
Rename parameters in docs and sources to match header declarations,
applied via dev/check_param_names.py --fix --check-src.

Remaining mismatches that could not be auto-fixed:
- arb_calc_isolate_roots: source uses blocks/block but doc uses
  found/interval. The ADD_BLOCK macro captures these names, blocking
  rename. Doc names are more descriptive and used in prose.
- _padic_poly_fprint_pretty, _padic_poly_print_pretty: val/len
  swap between header and source (fixed manually in previous commit)
- padic_poly_set_coeff_padic: cannot rename source 'x' to 'c'
  due to collision with local variable 'c'
arb: arb_set, arb_set_si, arb_set_ui, arb_set_d, arb_set_fmpz, and
arb_neg_round had x/y swapped relative to the rest of the arb module
(convention: y = output, x = input). Updated header, source, and bodies.

gr_poly: _gr_poly_div_series_newton and _gr_poly_div_series_invmul had
A/B swapped between header and source. Convention is (res, A, Alen, B,
Blen) where A is the dividend and B the divisor. Updated headers and
sources to match.

nmod_mat: nmod_mat_neg doc had A/B swapped vs header and source.
Updated doc to match.
@edgarcosta edgarcosta force-pushed the fix-param-names-batch1 branch from 4397734 to 5887f10 Compare March 12, 2026 05:30
@edgarcosta edgarcosta marked this pull request as ready for review March 12, 2026 05:43
@fredrik-johansson
Copy link
Copy Markdown
Collaborator

This looks fine to merge once ca_vec_printn has been fixed.

@edgarcosta
Copy link
Copy Markdown
Member Author

edgarcosta commented Mar 15, 2026

Fixed, sorry for the double error!

@fredrik-johansson fredrik-johansson merged commit 633ec99 into flintlib:main Mar 16, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants