Implement state variables in rust #86

dqnykamp · 2024-01-04T17:55:17Z

No description provided.

Revert StateVarReference back to StateVar

…xtend source

packages/doenetml-worker-rust/src/CoreWorker.ts

siefkenj · 2024-01-08T03:49:39Z

packages/doenetml-worker-rust/doenetml-core/src/parse_json.rs

+impl From<serde_json::Number> for StateVarValue {
+    /// A json number will be converted to an integer if it is an integer.
+    ///
+    /// For this reason, it's important that we can convert StateVarValue::Integer to f64,


Are there cases where we'd want to use BigInt instead?

I haven't needed a BigInt yet, but I could imagine eventually folks will want it. I'm trying to build a structure where we can easily add additional data types.

packages/doenetml-worker-rust/src/CoreWorker.ts

packages/doenetml-worker-rust/doenetml-core/src/utils.rs

siefkenj · 2024-01-09T02:17:54Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+#[derive(Debug, Clone)]
+pub enum StateVariableUpdateRequest {
+    SetEssentialValue(EssentialStateDescription),
+    SetStateVar(ComponentStateDescription),


I find the term Essential to be a bit confusing here, since to me it means "must be there", but there might be EssentialStateVars that are not referenced and therefore not needed...

What about the term Primitive? We could have a PrimitiveValue and DerivedValue.

The term Essential is intended to convey that these are the only state variables needed to reconstruct the document state (as every other state variable depends on them). When we write state to a database, we need only save the essential state. I think if an EssentialStateVar is not referenced, then it isn't even created. (I'd have to review the code more carefully to be certain of this last claim.)

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

packages/doenetml-worker-rust/doenetml-core/src/lib.rs

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

siefkenj · 2024-01-09T04:53:18Z

packages/doenetml-worker-rust/doenetml-core/src/dependency.rs

+
+/// A DependencyInstruction is used to make a Dependency based on the input document structure
+#[derive(Debug)]
+pub enum DependencyInstruction {


DependencyRequest seems like a better name.

Except that I've been using "request" to mean requested values for state variables, as in DependencyUpdatesRequested and StateVariableUpdateRequest. (I think I'll change the former to DependencyUpdateRequest for consistency.)

At the moment, I'm not able to come up with a better name that wouldn't be confused with DependencyUpdateRequest, but I'm open to suggestions!

I don't think that is such a bad conflict. You have a DepGetRequest and DepSetRequest. Seems like a common distinction.

I tried changing this to DependencyRequest and renaming the other to DependencyValueUpdateRequest but then ended up reverting that change as it got too confusing due to the DependencyValueUpdateRequest then referring to a request_idx rather than an instruction_idx. With the DependencyValueUpdateRequest overlapping with the "instructions", it was too confusing to call the instructions "requests".

I'm happy to revisit, but it became too hard to disambiguate them for now.

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

siefkenj · 2024-01-09T05:31:52Z

packages/doenetml-worker-rust/doenetml-core/src/component/text_input.rs

+        }
+    }
+
+    fn get_action_names(&self) -> Vec<String> {


This is used for type checking, I assume?

I'm still not sure the best way to do this, but if you set up an enum and use

use strum_macros::{EnumString, EnumVariantNames}; use strum::VariantNames; #[derive(EnumString, EnumVariantNames)] #[strum("camelCase")] enum Actions { UpdateImmediateValue, UpdateValue }

then you can do Actions::VARIANTS to get a 'static vec of the string names.

Deferring until another PR.

packages/doenetml-worker-rust/src/CoreWorker.ts

packages/doenetml-worker-rust/doenetml-core/src/utils.rs

Dispatch and process updates

create separate function for resolving.

packages/doenetml-worker-rust/doenetml-core/src/component/text.rs

siefkenj

The rewritten function looks so much better!

siefkenj · 2024-01-09T22:35:00Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+                match &dep.source {
+                    DependencySource::StateVar {
+                        component_idx,
+                        state_var_idx,


Since you're only using these to create a new pointer, you could implement into StateVarPointer and not de-structure this.

I added this trait. The code is now shorter, but seems less explicit. I added some comments to explain what is going on. It seemed unnecessary to have a match since try_from did everything.

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

siefkenj · 2024-01-09T22:40:47Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+                            // so don't do anything to just continue the inner loop
+                            Freshness::Fresh => (),
+                            Freshness::Stale | Freshness::Resolved => {
+                                if !found_stale_or_resolved_dependency {


This is really tracking whether state_var_ptr has already been re-added to the stack. Maybe call this variable state_var_ptr_on_stack? And set it to false as the first thing you do in the loop.

Changed the variable name and added more comments. Hope it is clearer now.

siefkenj · 2024-01-09T22:42:20Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+        if !found_stale_or_resolved_dependency {
+            // All the dependencies of state_var_ptr are fresh.
+            // We calculate its value if a dependency has changed,
+            // else restore its previous value.


I still don't quite get this part. We might need to talk about it.

Changed the variable name and added more comments. Hope it is clearer now.

siefkenj · 2024-01-09T22:44:48Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+
+    {
+        let component = components[component_idx].borrow();
+        let state_var = &component.get_state_variables()[state_var_idx];


Can you explicitly type this. rust-analyzer is having trouble...

Something is wrong with your rust-analyzer! All of these types come directly from the assignment...

siefkenj · 2024-01-09T22:44:54Z

packages/doenetml-worker-rust/doenetml-core/src/state_var_calculations.rs

+    let component_idx = state_var_ptr.component_idx;
+    let state_var_idx = state_var_ptr.state_var_idx;
+
+    let dependency_instructions;


Can you explicitly type this. rust-analyzer is having trouble...

Strange how my rust-analyzer doesn't have any trouble here or at the later spots.
It should able to infer it from later code.
But, it doesn't hurt to add it.

packages/doenetml-worker-rust/doenetml-core/doenetml-derive/src/component_node.rs

packages/doenetml-worker-rust/doenetml-core/src/component/text.rs

dqnykamp added 11 commits December 30, 2023 12:43

create state variable framework

d0a3057

text state variables and component profiles

623e5c7

wip: base rendered text component off state variable

8ae3ecf

component state variable functions, state var reference

79a7c91

Remove ComponentStateVariables trait

1c92575

Revert StateVarReference back to StateVar

rename ComponentNodeBase to ComponentNodeStateVariables

6a669f5

implement draft algorithm to calculate state variables

57a7fdc

comments and small tweaks

35383bb

handle actions to update state variables and send back flat dast updates

24244e2

match public state variables from macros, creating a state variable e…

59df8c5

…xtend source

freshen stale renderer components iteratively to avoid stack overflow

66cb099

siefkenj reviewed Jan 8, 2024

View reviewed changes

dqnykamp added 2 commits January 7, 2024 23:05

resolve() in finally block

108fc21

Merge branch '0.7' into state-vars

be34810

siefkenj reviewed Jan 9, 2024

View reviewed changes

dqnykamp added 5 commits January 9, 2024 09:27

basic dispatch_action works

e6e440e

small updates

3061f99

break apart files

7775666

move function

7d6b462

renaming

df0b31f

siefkenj reviewed Jan 9, 2024

View reviewed changes

packages/doenetml-worker-rust/src/CoreWorker.ts Outdated Show resolved Hide resolved

siefkenj reviewed Jan 9, 2024

View reviewed changes

packages/doenetml-worker-rust/doenetml-core/src/utils.rs Outdated Show resolved Hide resolved

siefkenj and others added 4 commits January 9, 2024 13:52

Dispatch and process updates

6e61687

Set children on update

38dacea

Merge pull request #2 from siefkenj/drust

91798c6

Dispatch and process updates

rework freshen state variable algorithm.

78ace38

create separate function for resolving.

siefkenj reviewed Jan 9, 2024

View reviewed changes

packages/doenetml-worker-rust/doenetml-core/src/component/text.rs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/state-vars' into state-vars

0f5a2e9

dqnykamp added 2 commits January 9, 2024 15:39

Bug fixes for new freshen state algorithm

99d6d60

adding tailcall

708dd9a

siefkenj reviewed Jan 9, 2024

View reviewed changes

dqnykamp added 5 commits January 9, 2024 17:33

tweak case-insensitive comparison

b75eaba

fix panic! due to component borrow_mut() overlapping with borrow()

702a72c

use enum_dispatch rather than custom proc macros

339764b

tweaks to freshen_state_var

9e0e095

add standard component field via attribute macro

b7ca01d

siefkenj reviewed Jan 10, 2024

View reviewed changes

packages/doenetml-worker-rust/doenetml-core/doenetml-derive/src/component_node.rs Outdated Show resolved Hide resolved

siefkenj reviewed Jan 10, 2024

View reviewed changes

packages/doenetml-worker-rust/doenetml-core/src/component/text.rs Outdated Show resolved Hide resolved

dqnykamp added 3 commits January 10, 2024 11:34

minor tweaks

5d49903

add common component fields via struct rather than macro

8d6dcd3

remove unused import

4fabe57

dqnykamp marked this pull request as ready for review January 10, 2024 21:07

dqnykamp merged commit 8c39141 into Doenet:0.7 Jan 10, 2024
1 check passed

dqnykamp deleted the state-vars branch January 27, 2024 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement state variables in rust #86

Implement state variables in rust #86

dqnykamp commented Jan 4, 2024

siefkenj Jan 8, 2024

dqnykamp Jan 8, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 9, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 9, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024

siefkenj Jan 9, 2024 •

edited

Loading

dqnykamp Jan 10, 2024

siefkenj left a comment

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024

siefkenj Jan 9, 2024

dqnykamp Jan 10, 2024 •

edited

Loading

Implement state variables in rust #86

Implement state variables in rust #86

Conversation

dqnykamp commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siefkenj Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siefkenj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dqnykamp Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

siefkenj Jan 9, 2024 •

edited

Loading

dqnykamp Jan 10, 2024 •

edited

Loading