Skip to content

Document and export continuous_events and discrete_events #2567

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

Merged
merged 5 commits into from
Mar 28, 2024

Conversation

MasonProtter
Copy link
Contributor

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

I'd like a blessed API for fetching the callbacks of an ODESystem without reaching into internals, so I figure marking these functions as exported and giving them docstrings would be a good way to go about this. If there's a better, existing way to do that please let me know.

@MasonProtter MasonProtter changed the title Document and export continuous_events and discrete_events Document and export continuous_events and discrete_events Mar 23, 2024
@ChrisRackauckas
Copy link
Member

Seems reasonable to me. Add it to the docs?

@@ -221,6 +225,10 @@ equations can either all change unknowns (i.e. variables) or all change
parameters, but one cannot currently mix unknown variable and parameter changes within one
individual event.

Given an `AbstractSystem`, one can fetch its discrete events, and the discrete events of any
Copy link
Member

Choose a reason for hiding this comment

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

Add it to the system docs for the docstrings of the available functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I see, I missed that section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove the mention of it from Events.md and just have it in the system docs?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

@MasonProtter
Copy link
Contributor Author

Done, let me know if that's a sufficient level of detail. I'm not sure how much we should explain the structure of SymbolicContinuousCallback and SymbolicDiscreteCallback. Maybe just say something like "these encode the first element of the input Pair in the .eqs field and the second element of the input Pair in the .affect field"?

@ChrisRackauckas
Copy link
Member

Maybe just say something like "these encode the first element of the input Pair in the .eqs field and the second element of the input Pair in the .affect field"?

That would be good to add.

Looks like formatter is blocking tests.

@ChrisRackauckas
Copy link
Member

Odd docs build failure, I'm going to assume that was a freak event, but also do a pass over this repo soon to fix a few red checkmarks.

@ChrisRackauckas ChrisRackauckas merged commit f0ce857 into SciML:master Mar 28, 2024
@MasonProtter MasonProtter deleted the export-events branch March 28, 2024 12:06
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.

2 participants