Skip to content

Commit

Permalink
fix: make environments roundtrip as Z data (#1200)
Browse files Browse the repository at this point in the history
PR #1183 didn't include the logic for environments when collecting
the children of pointers. We didn't see the error because our tests
weren't checking for environment roundtrip through Z data.

Here we fix that bug and also enhance the tests to avoid future
regressions.
  • Loading branch information
arthurpaulino authored Mar 7, 2024
1 parent 2b2e5b1 commit 7ad45e6
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 11 deletions.
47 changes: 36 additions & 11 deletions src/cli/zstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
store::{expect_ptrs, intern_ptrs_hydrated, Store},
tag::Tag,
},
tag::ExprTag::{Env, Sym},
tag::ExprTag::Env,
};

use super::field_data::HasFieldModulus;
Expand Down Expand Up @@ -51,14 +51,8 @@ impl<F: LurkField> ZDag<F> {
z_ptr
}
RawPtr::Hash4(idx) => {
if let Tag::Expr(Env) = tag {
let [sym_pay, val_tag, val_pay, env_pay] = store.expect_raw_ptrs(*idx);
let sym = Ptr::new(Tag::Expr(Sym), *sym_pay);
let val = Ptr::new(
store.fetch_tag(val_tag).expect("Couldn't fetch tag"),
*val_pay,
);
let env = Ptr::new(Tag::Expr(Env), *env_pay);
if matches!(tag, Tag::Expr(Env)) {
let [sym, val, env] = store.expect_env_components(*idx);
let sym = self.populate_with(&sym, store, cache);
let val = self.populate_with(&val, store, cache);
let env = self.populate_with(&env, store, cache);
Expand Down Expand Up @@ -153,8 +147,14 @@ impl<F: LurkField> ZDag<F> {
match ptr.raw() {
RawPtr::Atom(..) => (),
RawPtr::Hash4(idx) => {
for ptr in expect_ptrs!(store, 2, *idx) {
feed_loop!(ptr)
if matches!(ptr.tag(), Tag::Expr(Env)) {
for ptr in store.expect_env_components(*idx) {
feed_loop!(ptr)
}
} else {
for ptr in expect_ptrs!(store, 2, *idx) {
feed_loop!(ptr)
}
}
}
RawPtr::Hash6(idx) => {
Expand Down Expand Up @@ -420,6 +420,15 @@ mod tests {
3 => Tag::Op2(Op2::try_from((rnd % Op2::COUNT) as u16 + OP2_TAG_INIT).unwrap()),
_ => unreachable!(),
};
if matches!(tag, Tag::Expr(ExprTag::Env)) {
let mut env = store.intern_empty_env();
for _ in 0..max_depth {
let sym = store.intern_user_symbol("foo");
let val = rng_interner(rng, max_depth - 1, store);
env = store.push_binding(sym, val, env);
}
return env;
}
if max_depth == 0 {
store.intern_atom(tag, Bn::from_u64(rnd.try_into().unwrap()))
} else {
Expand Down Expand Up @@ -475,6 +484,22 @@ mod tests {
});
}

#[test]
fn test_env_roundtrip() {
let store = Store::<Bn>::default();
let val = store.num_u64(1);
let sym = store.intern_user_symbol("a");
let env = store.intern_empty_env();
let env = store.push_binding(sym, val, env);
let mut z_dag = ZDag::default();
let z_env = z_dag.populate_with(&env, &store, &mut Default::default());
let store2 = Store::<Bn>::default();
let env2 = z_dag
.populate_store(&z_env, &store2, &mut Default::default())
.unwrap();
assert_eq!(store.hash_ptr(&env), store2.hash_ptr(&env2));
}

#[test]
fn test_filtered_dag() {
let store = Store::<Bn>::default();
Expand Down
11 changes: 11 additions & 0 deletions src/lem/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,17 @@ impl<F: LurkField> Store<F> {
}
}

pub fn expect_env_components(&self, idx: usize) -> [Ptr; 3] {
let [sym_pay, val_tag, val_pay, env_pay] = self.expect_raw_ptrs(idx);
let sym = Ptr::new(Tag::Expr(Sym), *sym_pay);
let val = Ptr::new(
self.fetch_tag(val_tag).expect("Couldn't fetch tag"),
*val_pay,
);
let env = Ptr::new(Tag::Expr(Env), *env_pay);
[sym, val, env]
}

/// Fetches an environment
pub fn fetch_env(&self, ptr: &Ptr) -> Option<Vec<(Ptr, Ptr)>> {
if *ptr.tag() != Tag::Expr(Env) {
Expand Down

1 comment on commit 7ad45e6

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Benchmarks

Table of Contents

Overview

This benchmark report shows the Fibonacci GPU benchmark.
NVIDIA L4
Intel(R) Xeon(R) CPU @ 2.20GHz
32 vCPUs
125 GB RAM
Workflow run: https://github.com/lurk-lab/lurk-rs/actions/runs/8182327561

Benchmark Results

LEM Fibonacci Prove - rc = 100

ref=2b2e5b140dcdde8e11ac30d308aab7dd0e2f1258 ref=7ad45e6a61ff1e44deab90a99853dbf553a4c744
num-100 1.45 s (✅ 1.00x) 1.45 s (✅ 1.00x faster)
num-200 2.76 s (✅ 1.00x) 2.77 s (✅ 1.00x slower)

LEM Fibonacci Prove - rc = 600

ref=2b2e5b140dcdde8e11ac30d308aab7dd0e2f1258 ref=7ad45e6a61ff1e44deab90a99853dbf553a4c744
num-100 1.86 s (✅ 1.00x) 1.84 s (✅ 1.01x faster)
num-200 3.05 s (✅ 1.00x) 3.05 s (✅ 1.00x slower)

Made with criterion-table

Please sign in to comment.