argmin icon indicating copy to clipboard operation
argmin copied to clipboard

propagate rand 0.9 xoshiro 0.7 deps

Open jean-pierreBoth opened this issue 8 months ago • 6 comments

updates deps. from_entropy replaces by from_os_rng (rand docs recommans chacha for portability...) The rest should be ok. all tests passed.

jean-pierreBoth avatar Mar 21 '25 14:03 jean-pierreBoth

Thanks for the contribution! I have a few minor questions.

stefan-k avatar Mar 22 '25 14:03 stefan-k

first i got rid of the feature, then i saw it was changed to serde , forgot to re-establish it

Le sam. 22 mars 2025 à 15:07, Stefan Kroboth @.***> a écrit :

@.**** commented on this pull request.

In crates/argmin/Cargo.toml https://github.com/argmin-rs/argmin/pull/580#discussion_r2008779862:

@@ -43,7 +43,7 @@ argmin-checkpointing-file = { path = "../argmin-checkpointing-file" } [features] default = [] wasm-bindgen = ["getrandom/js"] -serde1 = ["serde", "rand_xoshiro/serde1"]

Why is this feature removed instead of changed to rand_xoshiro/serde as mentioned in the changelog https://github.com/rust-random/rngs/blob/master/rand_xoshiro/CHANGELOG.md ?

— Reply to this email directly, view it on GitHub https://github.com/argmin-rs/argmin/pull/580#pullrequestreview-2708043236, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXM7NJ4VWVGOOEQAVKVOMD2VVVBLAVCNFSM6AAAAABZQFQ5IWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBYGA2DGMRTGY . You are receiving this because you authored the thread.Message ID: @.***>

jean-pierreBoth avatar Mar 23 '25 15:03 jean-pierreBoth

fn gen https://docs.rs/rand/latest/rand/trait.Rng.html#method.gen<T>(&mut self) -> T where StandardUniform https://docs.rs/rand/latest/rand/distr/struct.StandardUniform.html: Distribution https://docs.rs/rand/latest/rand/distr/trait.Distribution.html<T>, 👎Deprecated since 0.9.0: Renamed to random to avoid conflict with the new gen keyword in Rust 2024.

Alias for Rng::random https://docs.rs/rand/latest/rand/trait.Rng.html#method.random.

Le sam. 22 mars 2025 à 15:12, Stefan Kroboth @.***> a écrit :

@.**** commented on this pull request.

In crates/argmin/src/solver/simulatedannealing/mod.rs https://github.com/argmin-rs/argmin/pull/580#discussion_r2008780942:

@@ -511,7 +511,7 @@ where // 1 / (1 + exp((next_cost - prev_cost) / current_temperature)), // // which will always be between 0 and 0.5.

  •    let prob: f64 = self.rng.gen();
    
  •    let prob: f64 = self.rng.random();
    

Could you point me to the documentation where this change is described, please?

— Reply to this email directly, view it on GitHub https://github.com/argmin-rs/argmin/pull/580#pullrequestreview-2708045890, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXM7NIYLUF3GXNY3WXVSTL2VVVT5AVCNFSM6AAAAABZQFQ5IWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBYGA2DKOBZGA . You are receiving this because you authored the thread.Message ID: @.***>

jean-pierreBoth avatar Mar 23 '25 15:03 jean-pierreBoth

coompiler asked it during modifs but in fact uncessary. Rng : RngCore

Le sam. 22 mars 2025 à 15:11, Stefan Kroboth @.***> a écrit :

@.**** commented on this pull request.

In crates/argmin/src/solver/particleswarm/mod.rs https://github.com/argmin-rs/argmin/pull/580#discussion_r2008780757:

@@ -146,7 +146,7 @@ impl<P, F, R> ParticleSwarm<P, F, R> where P: Clone + SyncAlias + ArgminSub<P, P> + ArgminMul<F, P> + ArgminRandom + ArgminZeroLike, F: ArgminFloat,

  • R: Rng,
  • R: Rng + RngCore,

Could you explain why this is necessary, please?

— Reply to this email directly, view it on GitHub https://github.com/argmin-rs/argmin/pull/580#pullrequestreview-2708045624, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXM7NIKGAWASYFK6NAI2O32VVVQJAVCNFSM6AAAAABZQFQ5IWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBYGA2DKNRSGQ . You are receiving this because you authored the thread.Message ID: @.***>

jean-pierreBoth avatar Mar 23 '25 17:03 jean-pierreBoth

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.12%. Comparing base (b35808a) to head (091f8c8).

Files with missing lines Patch % Lines
crates/argmin-math/src/ndarray_m/random.rs 50.00% 2 Missing :warning:
crates/argmin/src/solver/simulatedannealing/mod.rs 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #580      +/-   ##
==========================================
- Coverage   92.13%   92.12%   -0.01%     
==========================================
  Files         177      177              
  Lines       23672    23672              
==========================================
- Hits        21810    21808       -2     
- Misses       1862     1864       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Mar 24 '25 16:03 codecov-commenter

Last pb is wasm. cargo tree provides: ring ahash etc blocks to getrandom 0.2 , ring does not plan to change dep see https://github.com/briansmith/ring/issues/2341 getrandom 0.3 changed features and command line (see on their site).

jean-pierreBoth avatar Mar 26 '25 11:03 jean-pierreBoth

Obsolete thanks to #607

stefan-k avatar Sep 12 '25 13:09 stefan-k