salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.14k stars 152 forks source link

thread 'some_test' panicked at '`execute` method for field `MyValT::md` invoked', #453

Open enomado opened 1 year ago

enomado commented 1 year ago

Hi. Trying to iterate like

    let a = MyVal::new(&db, vec![1.0]);
    let b = MyVal::new(&db, vec![2.0]);

    let p = m_merge(&db, a, b);

    for i in 0..10 {
        a.set_md(&mut db).to(vec![i as f64]);
        dbg!(p.md(&db));
    }
running 1 test
thread 'some_test' panicked at '`execute` method for field `MyValT::md` invoked', habanero/src/lib.rs:31:1
stack backtrace:
   0: rust_begin_unwind
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/core/src/panicking.rs:67:14
   2: <habanero::__MyValTMd as salsa_2022::function::Configuration>::execute
             at ./src/lib.rs:31:1
   3: salsa_2022::function::execute::<impl salsa_2022::function::FunctionIngredient<C>>::execute::{{closure}}
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/execute.rs:48:43
   4: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/core/src/panic/unwind_safe.rs:271:9
   5: std::panicking::try::do_call
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/std/src/panicking.rs:500:40
   6: __rust_try
   7: std::panicking::try
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/std/src/panicking.rs:464:19
   8: std::panic::catch_unwind
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/std/src/panic.rs:142:14
   9: salsa_2022::cycle::Cycle::catch
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/cycle.rs:43:15
  10: salsa_2022::function::execute::<impl salsa_2022::function::FunctionIngredient<C>>::execute
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/execute.rs:48:27
  11: salsa_2022::function::fetch::<impl salsa_2022::function::FunctionIngredient<C>>::fetch_cold
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/fetch.rs:87:14
  12: salsa_2022::function::fetch::<impl salsa_2022::function::FunctionIngredient<C>>::compute_value::{{closure}}
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/fetch.rs:38:69
  13: core::option::Option<T>::or_else
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/core/src/option.rs:1513:21
  14: salsa_2022::function::fetch::<impl salsa_2022::function::FunctionIngredient<C>>::compute_value
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/fetch.rs:38:34
  15: salsa_2022::function::fetch::<impl salsa_2022::function::FunctionIngredient<C>>::fetch
             at /home/sc/t/bur/rust_libs/salsa/components/salsa-2022/src/function/fetch.rs:20:13
  16: habanero::MyValT::md
             at ./src/lib.rs:31:1
  17: habanero::some_test
             at ./src/lib.rs:71:14
  18: habanero::some_test::{{closure}}
             at ./src/lib.rs:59:20
  19: core::ops::function::FnOnce::call_once
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/core/src/ops/function.rs:250:5
  20: core::ops::function::FnOnce::call_once
             at /rustc/065a1f5df9c2f1d93269e4d25a2acabbddb0db8d/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test some_test ... FAILED

All code, except Database(its from example) - https://gist.github.com/enomado/92170cf942e5dc1b00b6a5f439ff7047

xffxff commented 1 year ago

@enomado I believe the panic is the expected behavior of Salsa, as mentioned in issue #407. The issue lies in the code p.md(&db) within the loop, which continues to use a tracked struct after altering the inputs using a.set_md(&mut db).to(vec![i as f64]). To fix this, you should call m_merge again to get the newest tracked struct.

let a = MyVal::new(&db, vec![1.0]);
let b = MyVal::new(&db, vec![2.0]);

let p = m_merge(&db, a, b);

for i in 0..10 {
    a.set_md(&mut db).to(vec![i as f64]);
    let p = m_merge(&db, a, b);
    dbg!(p.md(&db));
}

However, I think that the error message needs improvement to avoid confusion. A panic message like "accessing a field of a tracked struct MyVal from an older revision" would be better.

enomado commented 1 year ago

@xffxff thanks! I thought tracked structure can store whole computation chain at its own. I think it must be covered in documentation. just like

dont expect tracked struct can recalculate itself without calling whole chain

it could clarify a lot :)

Btw, please, tell where I wrong. I want generalize input structs and tracked structs