Skip to content

Conversation

@jerebertho
Copy link
Contributor

@jerebertho jerebertho commented Dec 22, 2025

  1. This PR introduces the possibility to use long options of type --long-options in msolve. Some short options now have a long option equivalent (-v --verbose, -V --version, -l --linear-algebra, etc.). The help page has been reworked to display these alternative long options.

  2. It also introduces the possibility for the user to give the seed to initialize the pseudo-random number generator with option --random-seed (no equivalent short option is provided). If -1 is given, then time(0) is used as before. Otherwise, the input 32-bit nonnegative number N is used.
    Observe that the latter should only be used for debug purpose only.
    It also cleans the code as only one call to srand is performed now.

  3. A bug was introduced in positive characteristic with No more random linear form over Q #249 where the introduction of a random linear form would make msolve quit at some point without outputting the result. This PR fixes this behaviour.

@jerebertho jerebertho marked this pull request as draft December 22, 2025 14:02
@jerebertho jerebertho marked this pull request as ready for review December 22, 2025 14:45
@vneiger
Copy link
Contributor

vneiger commented Dec 22, 2025

Thanks for the enhancements! Suggestions:

  • using uint32_t for the seed, instead of int32_t
  • maybe something like "-rs" (random seed) instead of "-a", if possible?

@jerebertho jerebertho marked this pull request as draft December 22, 2025 19:59
@jerebertho
Copy link
Contributor Author

I updated the initial comment: msolve now accepts long options such as --verbose, --linear-algebra. The tentative -a for giving the random-seed has been removed and is now called --random-seed.

I think it is now ready for review.

@jerebertho jerebertho marked this pull request as ready for review January 7, 2026 14:30
Copy link
Contributor

@vneiger vneiger left a comment

Choose a reason for hiding this comment

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

Here are a few minor changes (the only real change is the suggestion of unsigned for seed).

switch(opt) {
case 0: /* no short option equivalent */
if (random_seed_flag == 1) {
*seed = strtol(optarg, NULL, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
*seed = strtol(optarg, NULL, 10);
*seed = strtoul(optarg, NULL, 10);

int32_t precision = 64;
int32_t refine = 0; /* not used at the moment */
int32_t isolate = 0; /* not used at the moment */
int32_t seed = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int32_t seed = 0;
uint32_t seed = 0;

real_pts_ptr,
gens,
initial_hts, unstable_staircase, nr_threads, max_pairs,
initial_hts, unstable_staircase, nr_threads, max_pairs,
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a mix of tabs and spaces here

if (info_level > 0) {
printf ("\nRestarting with a non-random linear form");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

(keep the previous: spaces and not tabs)

if (info_level > 0) {
printf ("\nRestarting with another random linear form");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

(keep existing: no tab)

@jerebertho
Copy link
Contributor Author

I think it is now ready for review.

@jerebertho
Copy link
Contributor Author

Rebased after merging of #256.

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.

2 participants