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

Document integrator.tdir as part of the integrator interface #81

Closed
ChrisRackauckas opened this issue Feb 17, 2018 · 11 comments
Closed

Comments

@ChrisRackauckas
Copy link
Member

@tkf noticed this.

@ChrisRackauckas
Copy link
Member Author

Should add set_t! and stuff too.

@tkf
Copy link
Contributor

tkf commented Feb 20, 2018

In case you are interested in documenting absolutely all fields of the integrator touched in DiffEqBase, it seems there are little bit more:

$ cd ~/.julia/v0.6/DiffEqBase/src
$ git grep -E 'integ[a-z]*\.' | sed -E 's#integ[a-z]*\.(\w+)#\nXXX \1\n#g' | grep -E '^XXX' | cut -d' ' -f2 | sort -u
f
iter
opts
sol
t
tdir
tprev
u
u_modified
uprev

It's actually great that most of them are mentioned in the document! But it looks like .iter and .u_modified (not u_modified! method) are also not mentioned in the document. Not sure if they have to be, though. Maybe they are just interface "internal" to JuliaDiffEq/*.jl?

@tkf
Copy link
Contributor

tkf commented Feb 20, 2018

Quick document for set_*! is in #86.

@ChrisRackauckas
Copy link
Member Author

ChrisRackauckas commented Feb 20, 2018

u_modified should be used through the method. iter I think is internal, though maybe it's harmless to document it. Really what we should expose is SciML/DiffEqBase.jl#54 instead.

So it looks liek we've covered everything, though maybe we should change a line in DiffEqBase somewhere for u_modified?

@tkf
Copy link
Contributor

tkf commented Feb 20, 2018

I think it's OK to not document them in user docs. Though documenting them it in dev docs and/or comment helps contributors.

u_modified should be used through the method.

You mean store to integrator.u_modified? How about read? For example, there is:

function initialize!(u,t,integrator::DEIntegrator,any_modified::Bool,
                     c::DECallback)
  c.initialize(c,u,t,integrator)
  any_modified || integrator.u_modified
end

You want to have u_modified(integrator) (is_u_modified? get_u_modified?) method? But INITIALIZE_DEFAULT does integrator.u_modified = false and I guess that's bad and should use u_modified!.

@ChrisRackauckas
Copy link
Member Author

But INITIALIZE_DEFAULT does integrator.u_modified = false and I guess that's bad and should use u_modified!

Yup, that should use u_modified!.

How about read? For example, there is:

Yeah, this is relying on u_modified being a field, which is standardized but not formalized. That could probably cause an issue.

@tkf
Copy link
Contributor

tkf commented Feb 22, 2018

So modification has to occur via methods and get access is done directly. I see.

@ChrisRackauckas
Copy link
Member Author

Modification is user-facing so that has a documented function. Checking for modification is not.

@tkf
Copy link
Contributor

tkf commented Feb 22, 2018

I suppose checking for .tdir is also not user-facing. (or is it?) So .tdir, .u_modified, and .iter will be documented in http://devdocs.juliadiffeq.org/latest/?

@ChrisRackauckas
Copy link
Member Author

Yeah, the devdocs probably need to document some of the integrator conventions that are used and turn that into a proper "dev interface".

@tkf
Copy link
Contributor

tkf commented Feb 22, 2018

Sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants