Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototype mt_setup #139

Closed
wants to merge 2 commits into from
Closed

Prototype mt_setup #139

wants to merge 2 commits into from

Conversation

ChrisRackauckas
Copy link
Member

All of ModelingToolkit was made just as standard Julia functions and types. This is nice because then it's very interactive, but sometimes you want to do something at compile time. This idea is a simple macro @mt_setup which replaces things (thing) that eval with the piece that generates the code. In the macro form, this then is transformed at compile time to just be a code generating macro from MT syntax.

To really make this useful, we'll want to make it so that way more than 1 function is allowed. For example, generate the ODE function and its Jacobian, and then build the ODEFunction (or ODEProblem). Maybe we could have a bit more DSL to it, but I kind of like that you can copy-paste what worked in the global scope and now stick it in a function with this. That's not true without the macro because of the evals.

@codecov
Copy link

codecov bot commented May 28, 2019

Codecov Report

Merging #139 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #139      +/-   ##
==========================================
+ Coverage    93.9%   93.95%   +0.05%     
==========================================
  Files          11       12       +1     
  Lines         361      364       +3     
==========================================
+ Hits          339      342       +3     
  Misses         22       22
Impacted Files Coverage Δ
src/ModelingToolkit.jl 75% <ø> (ø) ⬆️
src/mt_setup.jl 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cabfea...d6eff50. Read the comment docs.

Copy link
Contributor

@HarrisonGrodin HarrisonGrodin left a comment

Choose a reason for hiding this comment

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

This seems somewhat out of place for ModelingToolkit. Supporting additional interfaces seems to make it feel more like a DSL in its own right, rather than a tool for building DSLs. It seems like we should encourage solving this style problem through use of the function API, right?

@@ -0,0 +1,6 @@
macro mt_setup(expr)
MacroTools.postwalk(expr) do x
x == :ODEFunction ? :generate_function : x
Copy link
Contributor

Choose a reason for hiding this comment

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

I seem to be missing the purpose of this macro. Isn't this the exact same thing as a user building a system and writing generate_function instead of ODEFunction?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it is. So then do we document generate_function so that way they can use the macro?

Copy link
Contributor

Choose a reason for hiding this comment

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

generate_function is already exported. Which macro - @mt_setup?

Copy link
Member Author

Choose a reason for hiding this comment

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

what about doing this like a trait system, where functions can opt in to have a change in here? I think we want to keep the copy-pastability into the macro.

@ChrisRackauckas
Copy link
Member Author

The main purpose of this is because MT cannot be used inside of a function right now because of the eval. We need some way to just throw a quick macro on it to make it not tied to the global scope.

@HarrisonGrodin
Copy link
Contributor

Couldn't this be solved by making generate_function a higher-order function which returns a function, rather than an expression? We could even just use an @eval before returning from generate_function, I believe.

@ChrisRackauckas
Copy link
Member Author

The purpose is to not eval at all.

@HarrisonGrodin
Copy link
Contributor

It seems like @mt_setup is just the no-op macro, since replacing ODEFunction with generate_function isn't necessary:

macro mt_setup(expr)
    expr
end

Rather than supporting an additional API for something more general than ModelingToolkit, it seems like this should exist elsewhere (Base, a utility package, etc.).

@ChrisRackauckas
Copy link
Member Author

I think we are going to want something like this, but not with the hardcoded name change. I'll get an update to this which is more extensible for the next release, and then we can keep tweaking it.

@ChrisRackauckas ChrisRackauckas deleted the mt_setup branch July 30, 2019 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants