Move some functions to different modules

[bwohlberg]

Moved some functions from scico.util and scico.math to new module scico.array.

It's not clear whether it's worth keeping scico.math; function safe_divide could be moved to scico.array, and rel_res would perhaps make sense in scico.metric.

[codecov]

Codecov Report

Merging #175 (8e5a95c) into main (86be104) will increase coverage by 0.00%.
The diff coverage is 97.65%.

@@           Coverage Diff           @@
##             main     #175   +/-   ##
=======================================
  Coverage   92.20%   92.21%           
=======================================
  Files          48       48           
  Lines        3350     3352    +2     
=======================================
+ Hits         3089     3091    +2     
  Misses        261      261           

Flag	Coverage Δ
unittests	92.21% <97.65%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
scico/util.py	90.98% <90.00%> (-1.99%)	⬇️
scico/array.py	97.46% <97.46%> (ø)
scico/_generic_operators.py	91.85% <100.00%> (-0.04%)	⬇️
scico/blockarray.py	91.20% <100.00%> (ø)
scico/functional/_norm.py	100.00% <100.00%> (ø)
scico/linop/_convolve.py	86.45% <100.00%> (ø)
scico/linop/_diff.py	92.85% <100.00%> (ø)
scico/linop/_linop.py	97.97% <100.00%> (ø)
scico/linop/optics.py	95.72% <100.00%> (ø)
scico/loss.py	88.67% <100.00%> (ø)
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86be104...8e5a95c. Read the comment docs.

[Michael-T-McCann]

It's not clear whether it's worth keeping scico.math; function safe_divide could be moved to scico.array, and rel_res would perhaps make sense in scico.metric.

Agreed on rel_res.

Suggestion: remove safe_divide entirely. It's used rarely in the code, it's a one line function, and (to my eye) the one line is more expressive the the words "safe divide".

[bwohlberg]

Suggestion: remove safe_divide entirely. It's used rarely in the code, it's a one line function, and (to my eye) the one line is more expressive the the words "safe divide".

I see some value in keeping it, if only as a record of the efficient way to implement the one line. As alternative names, perhaps no_nan_divide, or nan_safe_divide?

[tbalke]

I tend to share @Michael-T-McCann's sentiment and would suggest removing save_divide entirely.

Unless this can be solved more generally and with a wrapper replacing function values in some region? Something like

# a/b but 0.0 where b==0
c = special_case((__div__, a, b), b==0, 0.0)

[bwohlberg]

I'm still unconvinced on removing safe_divide. For now, moved it to scico.array, renamed to no_nan_divide.

[lukepfister]

The safe_divide function could benefit from some improved documentation, explaining when/why it is necessary to use.

The problem that it solves is differentiating something like sin(x) / x at zero. it has a well defined derivative at zero, but autograd will return nan. this manifests to the user as bizarro nans where they "shouldn't" be, and they can be hard to track down and debug. (see the discussion jax-ml/jax#5039 (comment) and in the linked thread)

so i think this function is helpful, but discoverability is an issue. if i was a new user and i wanted to code up a sinc function, would i know to use safe_divide?

[Michael-T-McCann]

Executive summary: I think no_nan_divide has its uses and is worth keeping.

The safe_divide function could benefit from some improved documentation, explaining when/why it is necessary to use.

Something I did not realize until reading the linked comment. This is not a matter of, e.g., sin(x) / x being nan at zero (though this is true and needs to be handled). Rather, it is a matter of the way you would expect to handle that, i.e., jnp.where(x == 0, 1.0, jnp.sin(x) / x), still having a nan gradient at zero!

It is a nasty jax sharp edge. Unfortunately I don't think this function can save people from the sharp edge, but it's useful for people who already know about the sharp edge.