Created Stratification DSL. Required adding getter methods for lss, …

[neonWhiteout]

…lssv, etc to StockFlow.jl, several methods to Syntax.jl (incl infer_links, which is now exported), and a Stratification.jl file. Also created a Stratification tests file and added more tests to Syntax.

[neonWhiteout]

Beyond maybe some comments and formatting, this should be fine.

[neonWhiteout]

Composition DSL has been separated out into #85

[neonWhiteout]

We may want to discuss the use of global flags, which I also use in Composition. I'm unsure what the best practice is. We might just want to delete them altogether.

[Saityi]

This looks pretty good overall to me.

[Saityi]

Could a lot of this be put into code by separating out the steps as different functions, and having the top-level just be a series of calls to them? extract_strata_name_indices or some such, parse_stratify_block, etc.?

[neonWhiteout]

Maybe. I think it's pretty separated already.

[neonWhiteout]

I'm unsure how to separate it into functions without it having a ton of arguments. Maybe I do some packing/unpacking for calls.

[Saityi]

Yeah maybe it'd be a bit onerous given all of the params; it might be a pretty big refactoring task. Unless @Xiaoyan-Li thinks otherwise, maybe this can just stay as-is? It does have a lot of explanatory comments.

[Saityi]

I'd add I am liking the refactors I am seeing 😃

[neonWhiteout]

In the last few commits I've factored out a few things by now operating vectors

[Saityi]

We may want to discuss the use of global flags, which I also use in Composition. I'm unsure what the best practice is. We might just want to delete them altogether.

Are these vars something users of the library might want to set? If not, could we perhaps remove one of the branches and just choose one permanently? And if it is, would this make more sense as some sort of keyword argument, like a kwargs dictionary of feature flags?

[neonWhiteout]

Yeah kwargs are probably the way to go, I don't know why I didn't think of that.

[Saityi]

This seems to indicate this is a macro called stratify, but it's a function called sfstratify. Should a macro named stratify also be added that calls off to sfstratify?

[neonWhiteout]

Yeah that's the result of a typo. Originally, this was done using a macro called @stratify, but since I needed the definitions of the stockflow, I turned it into a function.

Similarly, composition was originally a macro @compose.

[Saityi]

Is there anything stopping these from being macros? I think we can still have parameters (definitions of the stockflow) unless I'm missing something.

julia> macro test_params(a, b, c, block)
         println(a); println(b); println(c); println(block)
       end
@test_params (macro with 1 method)

julia> @test_params 1 2 3 begin
         :test
       end
1
2
3
begin
    #= REPL[5]:2 =#
    :test
end

Or for the (strata, type, aggregate) syntax we could have a tuple passed in.

P.S. You may want to enclose stuff like @stratify in backticks -- ` -- if you'd like to avoid tagging random users... although those do look like pretty inactive users. 🤷🏻‍♀️

[neonWhiteout]

It may be me misunderstanding how macros work. I don't know how to give the stockflows as arguments without using an eval somewhere. Providing the variables they're bound to just makes them symbols. I got around this in an earlier draft by checking the Main namespace and grabbing the corresponding value for the symbol, but this seemed like bad practice.

[Saityi]

This is related to the change in #80 about returning a code block instead of a value. That was a really big mistake on my part to begin with, which I repeated above in my other comment in a haste to comment.

julia> macro params2(a, b, c, body)
         return quote
           println($a); println($b); println($c); println($body);
         end
       end
@params2 (macro with 1 method)

julia> @params2 xs ys zs begin
         :test
       end
[1, 2, 3]
[3, 4, 5]
["a", "b", "c"]
test

I think this goes back to a while back when I said macros should only rearrange code, not run stuff. By doing it this way, the quoted portion replaces the macro call, then the code that was generated is evaluated. With what I was doing before, the macro call is replaced by a value which is then evaluated, and eval(value) for any value is just the value.

Alas, I am not a good or experienced Julia programmer.

[neonWhiteout]

I'd need to extract values out of an expression, though. Don't know if there's a better way...

[neonWhiteout]

Can a macro take kwargs...?
Might want to keep the function for that

[Saityi]

I'd need to extract values out of an expression, though. Don't know if there's a better way...

If I understand you correctly, it may be helpful to look at the code in #80 to get the idea. You can still do stuff like call functions, map over values, assign local variables, etc. in a quote block: https://github.com/AlgebraicJulia/StockFlow.jl/pull/80/files#diff-4e1b9ec77867b43e29dfce6850ad2d9cab1373bef6eed4a38ac4b5c29f94c2a7R147

Can a macro take kwargs...?
Might want to keep the function for that

You're right, they cannot. Hmm. I'll have to think about it. It might be worth us discussing this with @Xiaoyan-Li around the final design decisions for the flags/kwargs.

[neonWhiteout]

Have you confirmed your macros in #80 work? I'm struggling to get this one working, it keeps saying the variable I'm calling it with is undefined. I suspect it's a scope issue, since I'm able to get a macro working if I declare it in the same file. That is, I'm wondering if it's trying to find a variable in the Stratification module, when it only exists in the testing file which calls it.

eg, if I have in Stratification.jl:

macro stratify(strata, type, aggregate, block)

   return (quote
        $strata
    end)


end

and in the testing file I have:

Weight Model = # insert definition here
@stratify WeightModel l_type ageWeightModel quote ... end

I get UndefVarError: WeightModel not defined. However, if I have in one file:

#!/bin/env julia

macro f(x)
    quote
    cos($x)
    end
end
 begin
x = pi
end

println((@f x))

Then this correctly prints -1.0.

I don't understand how you're able to give the block as an argument without it being undefined.

[neonWhiteout]

Ah, nevermind, I think I got it, I needed an esc.

[neonWhiteout]

If we want to hold off longer I can try generalizing to allow for any amount of arguments. Would need to revamp the stratification syntax, but beyond that shouldn't be too difficult.

Struggling with macros which take a variable number of arguments...

[neonWhiteout]

Have any good ideas for a syntax to generalize beyond a pullback with only 2?

figure it'd be something like [(a1, ..., an), (b1, ..., bm), ... (x1, ..., xk)] => t1, but I dunno if there's something better.

Also, converting all those cumbersome vector and dicts to a struct with all the information for each stock flow.

I've sunk a lot of time into this...

[neonWhiteout]

https://github.com/neonWhiteout/StockFlow.jl/tree/Multi_Stratification Generalized stratification. Now works for an arbitrary number of stockflows.

Significantly different from present stratification, however. Uses structs and a lot of broadcasting. Notation is as I described in the previous comment.

I'm thinking I port over the structs and some of the structure into this branch's stratify, then make a separate stratify macro for the generalized form. Currently using the name 'nstratify', because it can have 'n' inputs, but idk.

[neonWhiteout]

Little funny. When I call a macro with something like A => B, it interprets that as an expression. In a function call, it's bound to a variable. When I try getting the value of that variable inside another expression, it tries finding the variables for A and B, but it isn't defined. In effect, it's unwrapping the original expression.

As such, need to wrap it in another expression. Expr(:quote, my_var)

[neonWhiteout]

Generalized stratification. Now works for an arbitrary number of stockflows.

Please see here for both generalized stratification and a rework of Stratification. We object oriented now.

Need to write tests for generalized stratification, but all the old tests still pass (after slight modification to use the right types)

[neonWhiteout]

May want to close this in favor of #87. Started a new branch because it is quite different from this one, though it's based off of it.