Feature/componentarray input output#21
Conversation
…imization.jl into feature/typed-wiring-diagrams
tylerhanks
left a comment
tylerhanks
left a comment
There was a problem hiding this comment.
Overall a very nice PR. I think there are some small specifics to clean up that I left comments on.
| # Implement the cospan-algebra of dynamical systems. | ||
| module Optimizers | ||
|
|
||
| export pullback_matrix, pushforward_matrix, Optimizer, OpenContinuousOpt, OpenDiscreteOpt, Euler, |
There was a problem hiding this comment.
I think we should still support the matrix versions of these functions.
| export pullback_matrix, pushforward_matrix, Optimizer, OpenContinuousOpt, OpenDiscreteOpt, Euler, | ||
| simulate | ||
| export Optimizer, OpenContinuousOpt, OpenDiscreteOpt, Euler, | ||
| simulate, pullback_function, pushforward_function, isapprox |
There was a problem hiding this comment.
It's bad practice to export functions from Base. We should just define the version of isapprox we need in the relevant test file (it's not critical to the function of this package).
|
|
||
|
|
||
|
|
||
| """ pullback_function(f::FinFunction, v::Vector) |
There was a problem hiding this comment.
I think we want pullback and pushforward functions to return the functions that do the pushforward and pullback when applied to a vector (similar to the matrix versions). This is because usually we will be calling pushforward and pullback with the same finfunction but different vectors over and over. So it would be nice to generate the function for a single finfunction once and then be able to call that multiple times on different inputs. This also maintains consistency with the matrix version.
|
|
||
| end No newline at end of file | ||
| # Run a discrete optimizer the designated number of time-steps. | ||
| function simulate(f::Open{Optimizer}, d::AbstractUWD, x0::ComponentArray, tsteps::Int) |
There was a problem hiding this comment.
I can't fight the feeling that there is a simpler/cleaner way to do this. I'm not sure what it is, but I will think about it.
|
|
||
|
|
||
| # Test ComponentArray version of input/output on scalar variables | ||
| # Note that all variables must be exposed to use this i/o system |
There was a problem hiding this comment.
Why do all variables have to be exposed?
| dc2 = composite_of_optimizers | ||
|
|
||
|
|
||
| x2 = ComponentArray(a=11.0, b=22, c=33, d=44, e=55, f=66, g=77, h=88, i=99) |
There was a problem hiding this comment.
Why is there an i component here when there is no i variable in the UWD?
2 participants
This includes the changes from my other PR, plus: