sundials icon indicating copy to clipboard operation
sundials copied to clipboard

Maintenance/arkode UI

Open drreynolds opened this issue 10 months ago • 1 comments

This is a draft PR, that includes only the code changes to ARKODE to create a shared UI for all steppers. I'm opening this now, in its current form, so that the team can provide feedback.

Remaining items before this is a "final" PR:

  1. Reorganize the ordering of functions in the header and implementation files, to group them according to UI vs internal, and to separate the now-deprecated functions. I'm leaving them in their current locations to make the GitHub "diff" more usable.

drreynolds avatar Apr 18 '24 21:04 drreynolds

Overall, I this looks great and it will make creating a stepper significantly easier. I think most or all of my requests are very minor changes.

Thanks for all of the suggestions where I forgot to include ark_mem when calling arkProcessError

I do have a little bit of concern that users will find all of the deprecation messages they are about to get very annoying (if it breaks their builds due to strict treatment of warnings for example), but I suppose there is not really a good way of avoiding that unless we want to support the old functions indefinitely.

I think it will be fine; if it takes a cluttered build log to convince users to eventually update their codes, then so be it. Another good reason to explicitly mark them as deprecated now is that it's a very clear indicator of which pieces should not be included when creating new ARKODE steppers.

I have not made it through every file (in particular the examples), but I looked at all of the main pieces.

drreynolds avatar Apr 26 '24 18:04 drreynolds

Revised plan:

  1. Once everyone is happy with this PR as-is, we can go ahead and merge.
  2. In a very-easy-to-review subsequent PR, I'll adjust the order of the contents in the source files to group them more logically into "deprecated", "user-facing", "internal", and "stepper routines provided to ARKODE" sections.

drreynolds avatar May 06 '24 18:05 drreynolds