-
Notifications
You must be signed in to change notification settings - Fork 191
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
Simulation parameters and prettier show methods #639
Conversation
Codecov Report
@@ Coverage Diff @@
## master #639 +/- ##
=======================================
Coverage 78.11% 78.11%
=======================================
Files 119 119
Lines 2326 2326
=======================================
Hits 1817 1817
Misses 509 509 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Let's get rid of model.parameters
ASAP. BoundaryFunction
also needs parameters.
BTW, the progress function does not have to run on the GPU, which means that flexible callable objects can be used which have embedded parameters. This is the way I envision progress function development moving forward. We may want eventually to have complex progress functions for fancy logging / in-terminal plotting, eg. A |
Ah interesting yeah a callable object would have worked as well. We should change iscallable(f) = !isempty(methods(f))
...
iscallable(sim.progress) && sim.progress(sim) See: https://discourse.julialang.org/t/how-to-determine-if-an-object-is-callable/2897 Although to have a callable object with parameters you'd have to define an new type right? (As in https://docs.julialang.org/en/v1/manual/methods/#Function-like-objects-1) Either way, would be good to move the discussion to an issue on getting rid of |
Would a blacklisting strategy work better than whitelisting? What kind of |
This PR allows simulations to hold parameters, useful if you need to access stuff from the
progress
function (e.g. see ali-ramadhan/Atmosfoolery.jl#63) going with the idea that we'll probably get rid ofmodel.parameters
in favor of more local parameters.It also cleans up show functions for models and simulations.