[Octave-patch-tracker] [patch #10278] GSoC 2022: add support for sparse computations in ode15{i, s} using Octave classes , making dependency on KLU optional.

[Sarrah Bastawala]

Follow-up Comment #5, patch #10278 (project octave):

[comment #3 comment #3:]
> @mmuetzel thanks for looking into this.
> 
>  - I don't have time right now to respond to all your comments, I hope maybe
@sarrah can?
 
Yes, thank you @mmuetzel and @cdf for your comments and looking into this. 


For @mmuuetzel 's comments : 
[comment #2 comment #2:]
> * I can't see where SUNDIALS_EXPORT is defined. What is it supposed to do?

As @cdf was kind to point out, SUNDIALS_EXPORT is a macro in the C API of
SUNDIALS and not defined by me. It is used to expose the replacement functions
created by me (which replace what would ideally be SUNDIALS functions) to the
rest of the SUNDIALS API.  

> * CamelCase in function names is discouraged in Octave's sources. Use all
lower case with underscores as separators (unless there is a good reason to
deviate from that convention here).
> * Many functions have names that contain "OCT" or "_Octave". Would it make
sense to move them to the `octave` namespace instead?

Regarding the above two points, as @cdf pointed out, most of these are with
regards to using the SUNDIALS C API explained above, as those functions that
are written in C++, still have to be called by the  C API, and are hence
exposed by `#extern C`. Hence I too do not think using such C++ functionality
will be possible.

> * Remove the include guard `__CONFIG_H_INCLUDE_GUARD` from `__ode15__.cc`.
`.cc` files aren't supposed to be included in other files normally. Is this
different for this file?
> * Revert the white space changes in your patch for the pre-processor macros.
The hash `#` is supposed to be in column 1. Spaces for indentation follow
after the `#`. Use two spaces for indentation.
> * Avoid using old-style cast in NV_CONTENT_C. Use `reinterpret_cast`
instead. Or does that not work for some reason here?
> * Use a space between function name and following opening parenthesis.
 
With regards to these points, this patch is a slightly older version, and I
did make some changes to the code after this especially to accommodate the
Coding Style. These are stored on the git repository mentioned by @cdf and the
changes are much clearly discussed by me at :
https://octave.discourse.group/t/advice-for-final-patch-submission-from-github-mirror/3232/3?u=sarrah-basta


If you could have a look at these it would be great. I will myself try
ensuring that I have followed all the style guide comments you posted about
and try rebasing my code on a current head on the default branch of the git
mirror and reply soon.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?10278>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/