mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

Add NAND and NOR number operations

Open Kryonn opened this issue 2 months ago • 19 comments

I was taking a look at logical operations and noticed that there is no function for NAND and NOR operations. This type of operation is most commonly used in projects related to digital systems, but since we have the logic category, I thought it would be a good idea to add these two to make it more complete. For now, I've only done this for number variables, but the idea is to expand the types of variables with subsequent PRs.

Kryonn avatar Oct 21 '25 17:10 Kryonn

Thanks @Kryonn! It makes sense to add two two variations too.

For now, I've only done this for number variables, but the idea is to expand the types of variables with subsequent PRs.

I indeed think though that we should not only support them for the plain numbers, but also for mathjs in general: add these two new functions to /src/function/logical.

josdejong avatar Oct 22 '25 09:10 josdejong

@Kryonn I see you've now implemented nor for all datatypes now. I guess implementing nand is on your todo list too.

Just to be sure: when implementing functions, there are a couple of extra things that need to be taken care of. This is documented here and may be of help: https://github.com/josdejong/mathjs/blob/develop/README.md#implementing-a-new-function

josdejong avatar Oct 29 '25 10:10 josdejong

Just to be sure: when implementing functions, there are a couple of extra things that need to be taken care of. This is documented here and may be of help: https://github.com/josdejong/mathjs/blob/develop/README.md#implementing-a-new-function

I decided to start with the NOR function, and then do the NAND function. So, for the NOR function, I checked and applied the steps mentioned. However, when I went to test the function, I got two errors that I can't seem to resolve. The first error was related to the return of the toTex function. I didn't quite understand how we define the return for a given function. The second problem was related to the operation between two sparse matrices. I couldn't quite understand the error message. Could you help me?

error message
26 passing (27ms)
  2 failing

  1) nor
       should LaTeX nor:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
actual expected

'\\mathrm{nor}\\left(1,\\2\\right)'

      + expected - actual

      -\mathrm{nor}\left(1,2\right)
      +\left(1\2\right)

      at Context.<anonymous> (file:///C:/Users/Kryoj/OneDrive/Documents/GitHub/mathjs/test/unit-tests/function/logical/nor.test.js:234:12)
      at process.processImmediate (node:internal/timers:485:21)

  2) nor
       SparseMatrix
         should nor sparse matrix - sparse matrix:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

  SparseMatrix {
    _datatype: undefined,
+   _index: [],
-   _index: [
-     0
-   ],
    _ptr: [
      0,
+     0,
+     0
-     1,
-     1
    ],
    _size: [
      2,
      2
    ],
+   _values: []
-   _values: [
-     true
-   ]
  }

      + expected - actual

       {
         "_datatype": [undefined]
      -  "_index": []
      +  "_index": [
      +    0
      +  ]
         "_ptr": [
           0
      -    0
      -    0
      +    1
      +    1
         ]
         "_size": [
           2
           2
         ]
      -  "_values": []
      +  "_values": [
      +    true
      +  ]
       }

Kryonn avatar Oct 30 '25 13:10 Kryonn

@Kryonn do you still need help? I did run the tests on the latest version of this PR but do not get the errors you report above (only two errors related to documentation).

josdejong avatar Oct 31 '25 16:10 josdejong

@Kryonn do you still need help? I did run the tests on the latest version of this PR but do not get the errors you report above

In the end, I managed to solve it. Now, all that's left is to implement the NAND function.

(only two errors related to documentation).

Okay! I'll check it carefully and correct it.

Kryonn avatar Nov 01 '25 00:11 Kryonn

I finished the NOR function, it’s now working correctly and without any documentation issues.

As for the NAND function, I tried to implement it, but I wasn’t able to fix the test errors. From what I can tell, it would be necessary to create new matrix function application algorithms (matAlgoXXXX), which seems a bit complicated.

So, I’ll wrap up my contribution here. I hope I was able to help, and thank you for your support!

Kryonn avatar Nov 05 '25 02:11 Kryonn

I have no objection to adding nand and nor functions to mathjs, but is there any motivation for writing new internal code as opposed to just implementing them internally as not(and(x, y)) and not(or(x, y))? is it the inefficiency of passing over array arguments twice, or something else? If the former is the only real concern, then I'd recommend implementing only the matrix algorithms directly, with or without the matAlgo functions as appropriate, and forwarding all other arguments to not(and(x,y)) or not(or(x,y)). In particular, since x nor y === (not x) and (not y) the "nor" matrix by scalar operations should short-circuit when the scalar is true like the "and" ones do when the scalar is false. (And similarly, the matrix implementation for "or" is actually your best template when implementing L nand M === (not L) or (not M).

Finally, my writing those expressions above makes me wonder whether nand and nor should be added as named infix operators like or and and? Again, I don't see any reason why they should be treated differently from the other logical operations, so I would recommend doing so.

@Kryonn, looking forward to your feedback on these points, and whether based on them you want to restart your efforts to get these operations into mathjs? If not, please be aware that due to limited time on the part of the core mathjs team, this PR would likely be converted to a discussion on adding "nand" and "nor" to mathjs, with a pointer to the closed PR as a starting place for anyone who is motivated to take up the effort to add these operations. It's just unfortunate that this PR as it stands isn't quite ready for merging (and in particular I have a tough time imagining merging nor without nand).

gwhitney avatar Nov 06 '25 04:11 gwhitney

I have no objection to adding nand and nor functions to mathjs, but is there any motivation for writing new internal code as opposed to just implementing them internally as not(and(x, y)) and not(or(x, y))? is it the inefficiency of passing over array arguments twice, or something else? If the former is the only real concern, then I'd recommend implementing only the matrix algorithms directly, with or without the matAlgo functions as appropriate, and forwarding all other arguments to not(and(x,y)) or not(or(x,y)). In particular, since x nor y === (not x) and (not y) the "nor" matrix by scalar operations should short-circuit when the scalar is true like the "and" ones do when the scalar is false. (And similarly, the matrix implementation for "or" is actually your best template when implementing L nand M === (not L) or (not M).

I actually thought about doing it that way, but it seemed a bit too easy to be true. Anyway, I rewrote the NOR function and implemented the NAND function, both built by applying the OR and AND functions followed by the NOT function.

Kryonn avatar Nov 06 '25 15:11 Kryonn

Excellent. With this, we will definitely get this PR over the finish line. Here is a checklist of items that remain before final review:

  • [ ] Add nor/nand to test/typescript-tests/testTypes.ts
  • [ ] Add nor/nand to the list of named infix logical operators in the mathjs expression language, parallel to or/and
  • [ ] Add parsing tests for the new infix operators
  • [ ] Avoid temporary matrices in the matrix versions of nor/nand (i.e., just for matrices, implement nor roughly the way you had, but with short-circuiting when the scalar is true in the matrix-by-scalar case, and implement nand on matrices directly - the problem with not-and for matrices is the "and" matrix gets made, then a copy with every entry negated gets made, and then the intermediate "and" matrix is just immediately thrown away. For large matrices that's a lot of extra work/memory thrashing.)
  • [x] Include nand in your new, and much appreciated, test/unit-tests/plain/number/logical.test.js

Please let us know which of the above you can tackle and plan to do (the more of them you do, the quicker this will get in, but we will be able to get around to all of them eventually now that the PR is this far along). Thanks so much!

gwhitney avatar Nov 06 '25 16:11 gwhitney

I will do these tasks

  • [x] Add nor/nand to test/typescript-tests/testTypes.ts
  • [x] Add nor/nand to the list of named infix logical operators in the mathjs expression language, parallel to or/and
  • [x] Add parsing tests for the new infix operators
  • [ ] Avoid temporary matrices in the matrix versions of nor/nand (i.e., just for matrices, implement nor roughly the way you had, but with short-circuiting when the scalar is true in the matrix-by-scalar case, and implement nand on matrices directly - the problem with not-and for matrices is the "and" matrix gets made, then a copy with every entry negated gets made, and then the intermediate "and" matrix is just immediately thrown away. For large matrices that's a lot of extra work/memory thrashing.)
  • [ ] Include nand in your new, and much appreciated, test/unit-tests/plain/number/logical.test.js

Kryonn avatar Nov 07 '25 09:11 Kryonn

Thanks for the further updates. I take it from your previous post that you plan to leave the matrix optimizations, and extending the plain number logical tests to nand, to the mathjs maintainer crew? (Just checking for definiteness, I wasn't clear whether maybe you made your post a checkbox list too so you could check them off as they were done?)

In any case, let's get these points finished off and move this PR ever closer to merging. Thanks!

True, I hadn't thought about it that way. But I really only intend to do the tasks that are marked.

Kryonn avatar Nov 12 '25 00:11 Kryonn

That's fine, just looking for clarity. If you're able to handle the comments on what you've done so far, we should be able to get this PR merged before long.

gwhitney avatar Nov 12 '25 00:11 gwhitney

That's fine, just looking for clarity. If you're able to handle the comments on what you've done so far, we should be able to get this PR merged before long.

I have finished correcting all the problems mentioned in the comments. I think everything is fine now.

Kryonn avatar Nov 12 '25 11:11 Kryonn

Excellent, thanks! Your parsing tests now show all of the grouping distinctions. However, they don't yet test the precedence, in that it just so happens in every one of the tests the higher-precedence operator, when there was a difference, happened to end up on the left. The unit tests need examples where the precedence causes grouping on the right, e.g. A or B nor C parses as A or (B nor C) (and so on for the other three combos where the higher-precedence operator is on the left).

Or actually, it would be simpler and better to just have tests that show that nand is lower-precedence than xor and nor is higher-precedence than xor:

  • Every operator should be tested against the ones immediately higher and lower precedence than it. Note that by this principle, now that the precedences of nand and nor have been corrected, please change the other precedence tests you have to be between nand and conditional, and between nor and bitwise or, since those are now the adjacent pairs.
  • Checking nand vs xor and then xor vs nor eliminates need to check nand vs nor, or vs nor, etc. etc., so actually fewer tests.

Note you can leave in all the tests you have so far -- there's no problem with them -- but tests that demonstrate the proper precedence of each vs xor are needed, since right now there don't happen to be any tests that prove proper precedence in the whole nand - xor - nor range. Thanks!

gwhitney avatar Nov 12 '25 16:11 gwhitney

@Kryonn did you see the last comment from Glen? Is it possible for you to have a last look into the tests?

josdejong avatar Nov 26 '25 09:11 josdejong

@Kryonn did you see the last comment from Glen? Is it possible for you to have a last look into the tests?

I'm still going to change things, I'm at the end of the semester at college and it's a bit hectic.

Kryonn avatar Nov 28 '25 19:11 Kryonn

No problem, thanks for the update 👍 . Good luck with college this semester.

josdejong avatar Dec 01 '25 10:12 josdejong

I’ve finished fixing the tests. The NAND–OR, NAND–AND, NAND–NOR, OR–NOR, and OR–AND tests are now actually verifying operator precedence (with the higher-precedence operator on the right). In addition, I added tests to verify precedence between NAND and Conditional, NAND and XOR, NOR and Bitwise OR, and NOR and XOR.

Kryonn avatar Dec 09 '25 15:12 Kryonn

OK, looks solid. I will go ahead and finish the last tasks now (plus the admin stuff like updating HISTORY). If I don't run into any snags, this should merge into develop soon.

gwhitney avatar Dec 09 '25 17:12 gwhitney

Just to update everyone: I extended the tests for plain/number/logical to include nand. I started work on the matrix shortcutting, but then realized that the existing or logical function could also use better shortcutting in the matrix-scalar case (right now, it just computes or of the scalar with every matrix entry, but instead it can check if the scalar is true, and if so, return the all-true matrix; whereas if the scalar is false, it can simply convert every matrix entry to boolean. There's never a need to repeatedly call or. @josdejong, do you want that improvement in its own PR or may I just fold it into this one?). Then implementing that shortcutting for or revealed #3609, which needs to be fixed first for either the or shortcutting or the desired nand/nor shortcutting. And then investigating #3609 revealed #3608, which needs to be fixed first of all. So this will be on hold for just a bit while we work through the other issues. Thanks for understanding.

gwhitney avatar Dec 11 '25 17:12 gwhitney

All right, you can fix whatever needs fixing without rushing. If you find anything I need to correct, just comment on this issue.

Kryonn avatar Dec 11 '25 23:12 Kryonn

Thanks @Kryonn for the updates, and @gwhitney for looking into this. Let's try not to make this PR larger, it is already quite large. Glen can you do any fixes in this PR, but related improvements in a separate PR? Thanks

josdejong avatar Dec 17 '25 11:12 josdejong

Well at the moment this PR does nothing special for matrices, which means that it is creating at least one or two temporary matrices and throwing them away to get its result, which is very expensive for large matrices. So I don't think we want to merge this as is. Trying to put that in shape led to the discovery that e.g. "or" is not taking advantage of shortcutting nearly as much as it can, and of course I want to use "or" as the model for "nand". Once #3611 is merged and #3608 is fixed, I could certainly then do a PR that improves the shortcutting in "or" (also maybe "and", not certain yet), and then once that's in, base the matrix handling here on those improved versions. That way the only further change to this PR would be the shift away from fully naive Matrix handling as it has now, which i don't think we ever want to merge. It will just make this PR wait in line a little bit. But i don't think I including nand/nor is urgent.

What do you think?

gwhitney avatar Dec 17 '25 16:12 gwhitney

OK clear, lets keep it in one PR then, no problem :+1:

josdejong avatar Dec 19 '25 13:12 josdejong