esl / ice

Apache License 2.0
2 stars 4 forks source link

Add draft inefficient support for extensionality #47

Open lucafavatella opened 10 years ago

lucafavatella commented 10 years ago

This pull request merges pull request #46 (solve possible calc value returned by evaluation of var id) in order to get test 'extensional_expr_w_primops' working.

Actually merge of pull request #46 would not be strictly needed as the problem (or suspicious behaviour) in this case resides in the cache (tdtree), not the semantics: when the root node of a variable contains multiple missing dimensions (e.g. s and t) and a find is executed with only one known dimension (e.g. t), the cache returns calc. This behaviour may be triggered by the context perturbation operator '@', e.g. 'B where var B = A @ [t <-0];; var A = #.t. + #.s;; dim t <- 0;; dim s <- 0 end' (example written off the top of the head simplifying another example).

lucafavatella commented 10 years ago

I rebased this branch on master in order not to depend anymore on pull request #45 (function closure), and simplify in this way the review of this pull request and the evolution of pull request #45. I squashed the commit (update of CL types) by Pierre in another commit.

Apologies for any inconvenience caused.

lucafavatella commented 10 years ago

Rebased on current master. Also simplified history for easing review.

lucafavatella commented 10 years ago

Pierre, I added a TODO comment in the textensional module re unique string representation of dimensions before offloading to CL. Can you have a look at that?

lucafavatella commented 10 years ago

Perfect thanks.

lucafavatella commented 10 years ago

I tested this pull request with ets_cache branch by Ed https://github.com/esl/tea/tree/ets_cache 9a92f4b (1 month ago). The ets cache would not remove the need to have pull request #46 merged, as test extensional_expr_w_primops would still fail without that.

lucafavatella commented 10 years ago

This will be impacted by PR #54 as it changes the format of the primop AST node. If PR #54 is merged before this, this PR will require merge and update; maybe also deps (x2cl?) will require update.