I agree about lines 164-166 and have no opinion about the rank of constants.
Henry Rich On 2/24/2023 10:57 AM, Jan-Pieter Jacobs wrote:
I was looking at Raul's pull request for math/calculus here: https://github.com/jsoftware/math_calculus/pull/5 . This would simplify a lot of derivatives, especially ones involving sines and cosines, which cause higher order derivatives to explode in the current version. I've been granted push rights on the repo, but I'm not 100% sure I understand the code proposed, and I'm also not sure how to use Github's code review tools (in fact, not being a programming professional, I never did a code review at all). How is this supposed to work? Are the Github tools used in general for collaboration on jsoftware's addons? I'm doubting about the lines 164-166 ( https://github.com/rdm/math_calculus/blob/796ddac9de3863bf6746861624bb6aad8b70b9a2/calculus.ijs#L164), where it seems to me that in x atops y, when y happens to be -@verb, just discards x and -@, keeping only "verb". I think this should only happen if x is -, i.e. I think a check is missing. Secondly, for now, constants, zeros, ones and negative ones are considered only if they are rank 0 (see lines 173, 178-180, at https://github.com/rdm/math_calculus/blob/796ddac9de3863bf6746861624bb6aad8b70b9a2/calculus.ijs#L173). Would it hurt to also accept them at other ranks (e.g. 0: , 0"_), e.g. would it have unexpected effects on pderiv? deriv_jcalculus already accepts 0: and related primitives for deriv and intg (and could likely be extended to accept 0"_, 1"_, etc as well). Thanks for your observations. Jan-Pieter PS: if you're all too busy preparing the j9.4 release, no rush, it can wait. ---------------------------------------------------------------------- For information about J forums see http://www.jsoftware.com/forums.htm
---------------------------------------------------------------------- For information about J forums see http://www.jsoftware.com/forums.htm
