search

jcorbino / mole

Mimetic Operators Library Enhanced
GNU General Public License v3.0
25 stars 19 forks source link

[joss review] Algebra between operators defined on different grids #13

Closed jakelangham closed 4 months ago

jakelangham commented 7 months ago

While playing around with the MATLAB component of MOLE (as part of https://github.com/openjournals/joss-reviews/issues/6288), I noticed that it is possible to add or multiply operators that are constructed with respect to different grids.

e.g. grad(2, 10, 1) + grad(2, 10, 2)

It also seems to be possible to add together operators on uniform and non-uniform grids.

I'm not sure what this would mean exactly. Is ever desirable to be able to do this? If not then perhaps the code should throw an error in these cases.

jcorbino commented 6 months ago

You're correct @jakelangham . Non-unary operations involving operands constructed over different grids can be performed, and this is certainly not a justified scenario (not that I can think of). I don't think one can handle exceptions of that kind without defining the operators (currently simply sparse matrices) as objects of a class. This will be a fundamental change in the code, which I'm not opposed to, but it will certainly require a significant amount of time.

jakelangham commented 6 months ago

Yep, I suspected as much. Perhaps it might be beneficial to the library in the future, especially if there are other reasons why supplying additional class data with the operators turns out to be useful.

In the short term, I would recommend that the documentation is supplied with a warning about this. Of course it's not likely to cause users problems in simple scenarios, but in e.g. a more complex code testing various grid refinements, one could foresee it leading to bugs down the line.

jcorbino commented 4 months ago

@jakelangham - I added this to the README file:

NOTE: Performing non-unary operations involving operands constructed over different grids may lead to unexpected results. While MOLE currently allows such operations without throwing errors, users must exercise caution when manipulating operators across different grids.

And I also added that note to the manual.

I think the only pending issue now is this one: https://github.com/jcorbino/mole/issues/12

jakelangham commented 4 months ago

@jcorbino Fantastic - closing this one.