Decide what arithmetic operations are allowed on spectra

[brechmos-stsci]

Need to be able to do arithmetic calculations and operations on Spectrum1D objects and on multiple Spectrum1D objects. Maybe easiest to start out with simple arithmetic operations.

[SaOgaz]

What arithmetic stuff do we want here? @brechmos-stsci and I have brainstormed addition, subtraction, some stats (mean, median, standard deviation), integrated flux

Do any of these make sense for spectra:
multiplication, division, power

Any others I don't have?

@nmearl @stscicrawford

[nmearl]

Even though we only have an arithmetic test that tests addition -- subtraction, division, and multiplication should work.

Currently, those are the only operator overloads implemented. You'd have to override __pow__ to get exponentials. You can see how it's currently implemented here.

[brechmos-stsci]

@SaOgaz and I were just talking about some of the implementation details and there are a couple of questions... For example, with addition, and assuming s1 and s2 are Spectrum1D objects, would you like:

1. s1.add(s2) so s2 is added into s1

2. s3 = s1.add(2) (probably doesn't make as much sense)

3. s3 = s1 + s2 (the obvious)

4. s3 = spec_arith.add(s1, s2) (not necessarily for adding but maybe spec_arith.mean(s1) ?)

Then.... given an answer there, there are a few options on implementation whether some functionality should be added to the Spectrum1D class, or as a Mixin or in a separate arithmetic type of class.

[crawfordsm]

There is also the read the docs page here:
http://specutils.readthedocs.io/en/latest/arithmetic.html

[nmearl]

Note that the arithmetic operations are implemented in the NDArithmeticMixin. More operations would have to be implemented there. Although, "old" specviz did not support these other operations, so we can certainly punt this to a later sprint.

[nmearl]

@brechmos-stsci 2. and 3. are already supported. See @crawfordsm's link to the docs.

[brechmos-stsci]

Cool. @SaOgaz is going to look through the arithmetic operations and will get back here to see if we want any others in the short term. She will also look at the tests and make sure they are complete. If everything is done, then we can close this and move on.

[brechmos-stsci]

(you guys will have to be slightly patient as we come up to speed with what is here and all :) )

[SaOgaz]

Okay, I've gone over the current arithmetic stuff and have a decent handle on the operator stuff and what's happening with the WCS. So once we have the test data setup (as discussed in the tag up) I can go ahead an add some tests for the other operators etc. I don't see anything else when it comes to the already existing basic operators that needs to be done.

[eteq]

@SaOgaz - so do you think this can be closed, then? Or should we leave this open but re-name it "document more clearly how to do arithmetic operations" or something like that?

[SaOgaz]

I'd like to leave it open at least until the tests are done. Maybe then we can rename it to documentation.

[SaOgaz]

Current support for arithmetic consists of the four basic algebra operators: addition, subtraction, division, multiplication.

[nmearl]

Running list of possible operations to support in arithmetic.

• Absolute value
• Power of base 10 (inverse log base 10)
• Power of base e (inverse log base e)
• Inverse/reciprocal (values equal to zero are set to 0.0 in the inverse)
• Log base 10 (values less than or equal to 0.0 are set to -0.5)
• Multiply by a constant (constant is queried)
• Log base e (values less than or equal to 0.0 are set to -0.5)
• Add by a constant (constant is queried)
• Square root (values less than 0.0 are set to 0.0)
• Add another spectrum
• Subtract another spectrum
• Multiply by another spectrum
• Divide by another spectrum

[SaOgaz]

After some offline discussion (for milestone 0.4.0) this issue will only extend to making sure the test suite for these basic operators (with spectra and constants) is well rounded out. We can have more discussion here on other arithmetic operations we may want in specutils, which can then be opened as new issues.

[nmearl]

@SaOgaz Could you rename this issue to something like "Fill out testing infrastructure for basic arithmetic", and then open a new issue (something like "Implement extended arithmetic tasks") that we can use for further discussion?

[SaOgaz]

@nmearl, was the handling of unit conversions that would need an equivalence parameter discussed in the APE at all? I see that there's a project card about convenience functions for this sort of thing inside spectutils, but would we want to extend that to add etc. two spectra with different flux units making the equivalence call automatically, or leave it to the user to do the conversion prior to the add?

Chatted with @stscicrawford about this briefly and he said if we were to add this it would probably be a something we would worry about latter.

[nmearl]

I'm more inclined to leave it to the user. For us to provide this functionality, we would need to support both internally applying equivalencies during arithmetic operations, and allowing the user to supply a custom equivalency for use during arithmetic operations. This would need a bit of building out, I believe. The alternative is simply letting the user first convert the unit before doing any arithmetic, which seems reasonable. If there's a lot of user will for this, we can certainly implement it, but I think Steve's right in that there are probably more important things at this moment.

[SaOgaz]

So I haven't heard any discussion on this since the sprint. Are we content with the current allowed arithmetic operators, or are there others that people think should be added?

[eteq]

I'm punting this to the next-next release and calling this issue actually making the decision. We might end up with another issue on actually implementing it which might take longer...

[hcferguson]

I'm okay with requiring the user to deal with unit conversions now and considering that as a later enhancement, as long as it throws an error if the units aren't the same.

What is it doing if the WCSs aren't the same? Is it resampling or throwing an error there as well?

[nmearl]

@hcferguson There are a list of checks that get performed on the respective wcs objects, and depending on if one fails, will log an error and return without performing the operation.