Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error messages can be confusing #105

Open
aidanhs opened this issue May 12, 2018 · 1 comment
Open

Error messages can be confusing #105

aidanhs opened this issue May 12, 2018 · 1 comment

Comments

@aidanhs
Copy link

aidanhs commented May 12, 2018

I have this smallish example:

extern crate differential_dataflow;
extern crate timely;
type Timestamp = u64;
fn main() {
    use differential_dataflow::input::Input;
    use differential_dataflow::operators::group::Group;
    timely::execute(timely::Configuration::Thread, move |worker| {
        let (mut handle, probe) = worker.dataflow::<Timestamp, _, _>(|scope| {
            let (handle, data) = scope.new_collection();
            let probe = data
                .group(|_group, log_ids, out| {
                    let log_id: u64 = log_ids.iter().map(|&(&ts, &diff)| ts).max().unwrap();
                    out.push((log_id, 1)) 
                })
                .inspect(|&((group, log_id), _time, _diff)| {
                    println!("{} {}", group, log_id)
                })
                .probe();
            (handle, probe)
        });

        let (group, log_id) = (4, 1004);
        handle.advance_to(0);
        handle.insert((group, log_id));
        handle.close();
        worker.step_while(|| !probe.done());
    }).unwrap();
}

But when trying to compile it gives me 6 errors of two varieties:

error[E0277]: the trait bound `&_: differential_dataflow::Diff` is not satisfied
   --> src/timely.rs:131:40
    |
131 |             let (handle, data) = scope.new_collection();
    |                                        ^^^^^^^^^^^^^^ the trait `differential_dataflow::Diff` is not implemented for `&_`
    |
    = help: the following implementations were found:
              <i32 as differential_dataflow::Diff>
              <isize as differential_dataflow::Diff>
              <i64 as differential_dataflow::Diff>
              <differential_dataflow::difference::DiffPair<R1, R2> as differential_dataflow::Diff>

and

error[E0599]: no method named `insert` found for type `differential_dataflow::input::InputSession<u64, (_, u64), &_>` in the current scope
   --> src/timely.rs:146:16
    |
146 |         handle.insert((group, log_id));
    |                ^^^^^^

My standard debugging mechanism in cases like this is to add type annotations and see when there's a contradiction. But when you add the annotations to the closure params, like .group(|_group, log_ids: &[(&(_, &isize))], out: &mut Vec<_>| {, you get a different cryptic error message (maybe rust-lang/rust#41078?):

error[E0631]: type mismatch in closure arguments
   --> src/timely.rs:133:18
    |
133 |                 .group(|_group, log_ids: &[(&(_, &isize))], out: &mut Vec<_>| {
    |                  ^^^^^ ------------------------------------------------------ found signature of `for<'r, 's, 't0, 't1> fn(_, &'r [&'s (&u64, &'t0 isize)], &'t1 mut std::vec::Vec<(u64, {integer})>) -> _`
    |                  |
    |                  expected signature of `for<'r, 's, 't0, 't1> fn(&'r _, &'s [(&'t0 _, _)], &'t1 mut std::vec::Vec<(_, _)>) -> _`

In this case, the actual problem is that the &diff should be diff - both of the failed compiles contain hints if you look hard enough, but it's not easy to figure out. The correct place to add type annotations to help out inference is actually to pin down the diff type, i.e. scope.new_collection::<_, isize>().

I don't know if there's anything actionable here, but I thought I'd raise the issue. One thing I'd observe is that many of the examples and documentation show type annotations on worker.dataflow to pin down the timestamp type - I suppose one possible solution is to just make it so that the 'idiomatic' examples demonstrate a similar thing for the diff type.

@frankmcsherry
Copy link
Member

Interesting. I don't think I've ever run in to this before, but I obviously have some biases. The Diff type is currently hidden as a default generic parameter, and .. yeah not sure where the trade-off between "reveal as idiomatic to set" vs "hide to avoid issues" lives.

It would be great to have errors about type parameters earlier, and if you have any tips (or examples elsewhere) of putting more constraints on the types so that they bail earlier rather than with whole-program type inference errors (which I also have a hard time grokking), I'm all ears/eyes/etc.

I think one take-away is that the documentation and examples for differential aren't up to snuff yet. I OD'd on DD recently and am currently recuperating, but up for making that the next priority. :)

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

No branches or pull requests

2 participants