Skip to content

Commit

Permalink
Var API cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
mkeeter committed May 26, 2024
1 parent 184dc12 commit dbade79
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 36 deletions.
14 changes: 7 additions & 7 deletions fidget/src/core/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ pub use tracing::TracingEvaluator;

/// A tape represents something that can be evaluated by an evaluator
///
/// The only property enforced on the trait is that we must have some way to
/// recycle its internal storage. This matters most for JIT evaluators, whose
/// tapes are regions of executable memory-mapped RAM (which is expensive to map
/// and unmap).
/// It includes some kind of storage and the ability to look up variable
/// mapping. The variable mapping should be identical to calling
/// [`Function::vars`] on the `Function` which produced this tape, but it's
/// convenient to be able to look up vars locally.
pub trait Tape {
/// Associated type for this tape's data storage
type Storage: Default;

/// Retrieves the internal storage from this tape
///
/// This matters most for JIT evaluators, whose tapes are regions of
/// executable memory-mapped RAM (which is expensive to map and unmap).
fn recycle(self) -> Self::Storage;

/// Returns a mapping from [`Var`](crate::var::Var) to evaluation index
Expand Down Expand Up @@ -169,9 +172,6 @@ pub trait Function: Send + Sync + Clone {
/// This is underspecified and only used for unit testing; for tape-based
/// functions, it's typically the length of the tape,
fn size(&self) -> usize;

/// Returns a map from variable to index
fn vars(&self) -> &VarMap;
}

/// A [`Function`] which can be built from a math expression
Expand Down
44 changes: 25 additions & 19 deletions fidget/src/core/shape/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,8 @@ pub struct Shape<F> {
/// Wrapped function
f: F,

/// Index of x, y, z axes within the function's variable list (if present)
///
/// We could instead store an array of [`Var`]s and look them in with
/// [`self.f.vars()[&v]`](Function::vars), but it's more efficient to cache
/// them upon construction (because they never change).
axes: [Option<usize>; 3],
/// Variables representing x, y, z axes
axes: [Var; 3],

/// Optional transform to apply to the shape
transform: Option<Matrix4<f32>>,
Expand Down Expand Up @@ -100,9 +96,12 @@ impl<F: Function + Clone> Shape<F> {
&self,
storage: F::TapeStorage,
) -> ShapeTape<<F::PointEval as TracingEvaluator>::Tape> {
let tape = self.f.point_tape(storage);
let vars = tape.vars();
let axes = self.axes.map(|v| vars.get(&v));
ShapeTape {
tape: self.f.point_tape(storage),
axes: self.axes,
tape,
axes,
transform: self.transform,
}
}
Expand All @@ -112,9 +111,12 @@ impl<F: Function + Clone> Shape<F> {
&self,
storage: F::TapeStorage,
) -> ShapeTape<<F::IntervalEval as TracingEvaluator>::Tape> {
let tape = self.f.interval_tape(storage);
let vars = tape.vars();
let axes = self.axes.map(|v| vars.get(&v));
ShapeTape {
tape: self.f.interval_tape(storage),
axes: self.axes,
tape,
axes,
transform: self.transform,
}
}
Expand All @@ -124,9 +126,12 @@ impl<F: Function + Clone> Shape<F> {
&self,
storage: F::TapeStorage,
) -> ShapeTape<<F::FloatSliceEval as BulkEvaluator>::Tape> {
let tape = self.f.float_slice_tape(storage);
let vars = tape.vars();
let axes = self.axes.map(|v| vars.get(&v));
ShapeTape {
tape: self.f.float_slice_tape(storage),
axes: self.axes,
tape,
axes,
transform: self.transform,
}
}
Expand All @@ -136,9 +141,12 @@ impl<F: Function + Clone> Shape<F> {
&self,
storage: F::TapeStorage,
) -> ShapeTape<<F::GradSliceEval as BulkEvaluator>::Tape> {
let tape = self.f.grad_slice_tape(storage);
let vars = tape.vars();
let axes = self.axes.map(|v| vars.get(&v));
ShapeTape {
tape: self.f.grad_slice_tape(storage),
axes: self.axes,
tape,
axes,
transform: self.transform,
}
}
Expand Down Expand Up @@ -185,12 +193,12 @@ impl<F> Shape<F> {
}

/// Borrows the inner axis mapping
pub fn axes(&self) -> &[Option<usize>; 3] {
pub fn axes(&self) -> &[Var; 3] {
&self.axes
}

/// Raw constructor
pub fn new_raw(f: F, axes: [Option<usize>; 3]) -> Self {
pub fn new_raw(f: F, axes: [Var; 3]) -> Self {
Self {
f,
axes,
Expand Down Expand Up @@ -302,8 +310,6 @@ impl<F: MathFunction> Shape<F> {
axes: [Var; 3],
) -> Result<Self, Error> {
let f = F::new(ctx, node)?;
let vars = f.vars();
let axes = axes.map(|v| vars.get(&v));
Ok(Self {
f,
axes,
Expand Down Expand Up @@ -642,7 +648,7 @@ mod test {
let s = ctx.import(&s);

let s = VmShape::new(&mut ctx, s).unwrap();
let vs = Function::vars(s.inner());
let vs = s.inner().vars();
assert_eq!(vs.len(), 3);

assert!(vs.get(&Var::X).is_some());
Expand Down
13 changes: 12 additions & 1 deletion fidget/src/core/var/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,18 @@ use std::collections::HashMap;
/// same thing. To generate a "local" variable, [`Var::new`] picks a random
/// 64-bit value, which is very unlikely to collide with anything else.
#[allow(missing_docs)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[derive(
Copy,
Clone,
Debug,
Hash,
Eq,
PartialEq,
Ord,
PartialOrd,
Serialize,
Deserialize,
)]
pub enum Var {
X,
Y,
Expand Down
4 changes: 0 additions & 4 deletions fidget/src/core/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,6 @@ impl<const N: usize> Function for GenericVmFunction<N> {
fn size(&self) -> usize {
GenericVmFunction::size(self)
}

fn vars(&self) -> &VarMap {
&self.0.vars
}
}

impl<const N: usize> RenderHints for GenericVmFunction<N> {
Expand Down
4 changes: 0 additions & 4 deletions fidget/src/jit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,10 +900,6 @@ impl Function for JitFunction {
fn size(&self) -> usize {
self.0.size()
}

fn vars(&self) -> &VarMap {
Function::vars(&self.0)
}
}

impl RenderHints for JitFunction {
Expand Down
1 change: 1 addition & 0 deletions wasm-demo/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion wasm-demo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use fidget::{
context::{Context, Tree},
render::{BitRenderMode, RenderConfig},
shape::Bounds,
var::Var,
vm::{VmData, VmShape},
Error,
};
Expand Down Expand Up @@ -37,7 +38,7 @@ pub fn serialize_into_tape(t: JsTree) -> Result<Vec<u8>, String> {
/// Deserialize a `bincode`-packed `VmData` into a `VmShape`
#[wasm_bindgen]
pub fn deserialize_tape(data: Vec<u8>) -> Result<JsVmShape, String> {
let (d, axes): (VmData<255>, [Option<usize>; 3]) =
let (d, axes): (VmData<255>, [Var; 3]) =
bincode::deserialize(&data).map_err(|e| format!("{e}"))?;
Ok(JsVmShape(VmShape::new_raw(d.into(), axes)))
}
Expand Down

0 comments on commit dbade79

Please sign in to comment.