open-spaced-repetition / fsrs-rs

FSRS for Rust, including Optimizer and Scheduler
https://crates.io/crates/fsrs
BSD 3-Clause "New" or "Revised" License
172 stars 19 forks source link

your/tensor library issue #232

Closed sineptic closed 1 week ago

sineptic commented 2 weeks ago

I'm trying to understand how use this library(to my project) and now try to compute params.

use fsrs::{DEFAULT_PARAMETERS, FSRS};

fn main() {
    let fsrs = FSRS::new(Some(&DEFAULT_PARAMETERS)).unwrap();

    let item2 = fsrs::FSRSItem {
        reviews: vec![
            fsrs::FSRSReview {
                rating: 1,
                delta_t: 1,
            },
            fsrs::FSRSReview {
                rating: 1,
                delta_t: 1,
            },
        ],
    };
    let item1 = fsrs::FSRSItem {
        reviews: vec![fsrs::FSRSReview {
            rating: 1,
            delta_t: 1,
        }],
    };

    let mut items = vec![item1; 62];
    items.push(item2);

    let _best_params = fsrs.compute_parameters(items, None).unwrap();
}

This code works, but when

    let mut items = vec![item1; 63];

program panics with this:

thread 'main' panicked at /home/sineptic/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ndarray-0.15.6/src/impl_methods.rs:1035:21:
ndarray: index 18446744073709551615 is out of bounds in array of len 19

18446744073709551615 = 2 ^ 64 - 1

last place in YOUR library before panic is model.rs:100 in this function:

pub(crate) fn init_stability(&self, rating: Tensor<B, 1>) -> Tensor<B, 1> {
    self.w.val().select(0, rating.int() - 1)
}

And whats interesting, if items len < 64, this function never invoces(in my example)

L-M-Sherlock commented 2 weeks ago

I guess it's an issue from upstream (the burn crate).

sineptic commented 2 weeks ago

Maybe, i dont dont know this lib, but as i say, on 63 items this(in your codebase) function isn't ivoked (sorry for my English)

L-M-Sherlock commented 2 weeks ago

After debugging your code, I find out the problem.

  1. The first review's delta_t should be 0 because it's the first review, so the delta_t since the last review doesn't exist.
  2. The FSRSItem should include two FSRSReview at least, because it consists of the feature part and label part.
sineptic commented 2 weeks ago
  1. What if I create card today, but first review is tomorrow?
  2. what are feature part and label part?
L-M-Sherlock commented 2 weeks ago
  • What if I create card today, but first review is tomorrow?

The delta_t review remains zero because FSRS does not take into account the card's creation time.

2. what are feature part and label part?

The final long-term review serves as the label portion, which the model predicts. The reviews preceding the last long-term review constitute the feature set upon which the prediction is based.

asukaminato0721 commented 1 week ago

Seems fixed, so close, feel free to reopen it or open a new issue.