Add print_stats method to ode and implement for CVODE integrator#165
Open
dmitry-kabanov wants to merge 1 commit intobmcage:masterfrom
Open
Add print_stats method to ode and implement for CVODE integrator#165dmitry-kabanov wants to merge 1 commit intobmcage:masterfrom
print_stats method to ode and implement for CVODE integrator#165dmitry-kabanov wants to merge 1 commit intobmcage:masterfrom
Conversation
03b60d0 to
dffe1a7
Compare
Collaborator
|
@dmitry-kabanov If you're still interested in working on this, would you be able to rebase this on the latest master? |
Author
dffe1a7 to
be54ff6
Compare
`CVODE` solver has function `CVodePrintAllStats` that prints statistics about integrator such as number of right-hand-side function evaluations and number of nonlinear solves.
be54ff6 to
651ef5c
Compare
Author
|
@aragilar I have rebased the PR on the latest master right now and have tested that it works with the following script: import numpy as np
from scikits_odes import ode
def rhs(t, y, ydot):
ydot[:] = -y
t0 = 0.0
y0 = [1.0]
s = ode("cvode", rhs, old_api=False)
sol = s.solve(np.linspace(t0, t0 + 1, 11), y0)
print(sol.values.y[:, 0])
s.print_stats() |
aragilar
reviewed
Jul 21, 2024
Comment on lines
+309
to
+313
| def print_stats(self): | ||
| if hasattr(self._integrator, "print_stats"): | ||
| self._integrator.print_stats() | ||
| else: | ||
| print(f"Method `print_stats` is not implemented for integrator {self._integrator}") |
Collaborator
There was a problem hiding this comment.
This should probably throw an exception or use logging.error, rather than printing.
Author
There was a problem hiding this comment.
AFAIR, I have modeled this method based on the method directly above it:
def get_info(self):
...
if hasattr(self._integrator, 'get_info'):
return self._integrator.get_info()
else:
return {}Will this work?
def print_stats(self):
if hasattr(self._integrator, "print_stats"):
self._integrator.print_stats()
else:
raise AttributeError(
f"Method `print_stats` is not implemented for integrator "
f"'{self._integrator}'")claude.ai says that "[this] approach is idiomatic and well-suited to the adapter pattern you're implementing" 😀
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
CVODEsolver has a function calledCVodePrintAllStatsthat prints statistics about the integration process, such as number of right-hand-side function evaluations and number of nonlinear solves.This PR adds a method
print_statsto theodeandCVodeclasses, such that user could print the above statistics in case, when they specifyCVodeas the integrator argument at the instantiation of theodeclass.CVodePrintAllStatsallows to print the statistics and two different formats (as a formatted table or as a CSV file), to a given file stream (FILE *object). Right now I have narrowed it to always printing statistics tostdoutas a formatted table.