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

Allow Modules as propagation method, remove :auto #59

Closed
wants to merge 1 commit into from

Conversation

goerz
Copy link
Member

@goerz goerz commented Nov 27, 2023

In preparation for allowing for ODE solvers via an optional dependency, we now allow (and encourage) passing modules as a propagation method.

Also, the method now must be explicitly specified, :auto is no longer an option. The heuristics for choosing a propagation methods are brittle, and it's probably a good idea for people to think about which method they should use anyway.

With optional dependencies (ODE) being added, automatically choosing a method becomes even weirder.

@goerz goerz added the breaking PRs that break compatibility label Nov 27, 2023
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (acb187c) 88.2% compared to head (9932d73) 88.4%.

❗ Current head 9932d73 differs from pull request most recent head d421db0. Consider uploading reports for the commit d421db0 to get more accurate results

Files Patch % Lines
src/propagator.jl 62.5% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master     #59     +/-   ##
========================================
+ Coverage    88.2%   88.4%   +0.2%     
========================================
  Files          26      26             
  Lines        1821    1788     -33     
========================================
- Hits         1605    1579     -26     
+ Misses        216     209      -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@goerz goerz force-pushed the no-auto-method branch 2 times, most recently from 01e483a to 9932d73 Compare November 27, 2023 18:38
In preparation for allowing for ODE solvers via an optional dependency,
we now allow (and encourage) passing modules as a propagation `method`.

Also, the `method` now must be explicitly specified, `:auto` is no
longer an option. The heuristics for choosing a propagation methods are
brittle, and it's probably a good idea for people to think about which
method they should use anyway.

With optional dependencies (ODE) being added, automatically choosing a
method becomes even weirder.
@goerz goerz closed this Nov 27, 2023
@goerz goerz deleted the no-auto-method branch November 27, 2023 19:22
@goerz goerz restored the no-auto-method branch November 27, 2023 19:22
@goerz goerz deleted the no-auto-method branch November 27, 2023 19:22
@goerz
Copy link
Member Author

goerz commented Dec 7, 2023

Not sure why this is marked as "closed": it was properly merged into master in d421db0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking PRs that break compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant