Add mul method to wmodes

[akhairna]

Hi @moble, @duetosymmetry, and @keefemitman. I am adding the __mul__ method to the waveform modes class. Using this we can implement multiplication between waveform modes of different array size. I am using the attributes of the first Wmodes object to define the l_max of product. Please suggest improvements for this.

---

📚 Documentation preview 📚: https://sxs--195.org.readthedocs.build/en/195/

[codecov-commenter]

Codecov Report

❌ Patch coverage is 14.08451% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.46%. Comparing base (5f87345) to head (e00133b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sxs/waveforms/waveform_modes.py	14.08%	61 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #195      +/-   ##
==========================================
- Coverage   43.78%   43.46%   -0.32%     
==========================================
  Files          86       86              
  Lines        6550     6621      +71     
  Branches     1070     1082      +12     
==========================================
+ Hits         2868     2878      +10     
- Misses       3424     3485      +61     
  Partials      258      258              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[keefemitman]

Thanks @akhairna! Just two minor comments/edit requests.

[akhairna]

@duetosymmetry, @keefemitman & @moble - The __mul__ method now uses multiplication_truncator to set the ell_max. Let me know if its default value - sum is a reasonable choice when the attribute isn't set!

[duetosymmetry]

I think there needs to be some clarification on how the two waveforms' multiplication_truncators are used. It needs to be well-documented what happens if they have two different values. I think the simplest would be to use the multiplication_truncator from the left argument, unless it's missing, in which case to use the one from the right argument; unless they are both missing, in which case to fall back on a default.

[moble]

This might be a good opportunity to close #21. Specifically, the _ModesTimeSeries subclass is used for a lot of stuff in memory.py which could maybe be simplified. We now have a good few of those features. It looks like add, subtract, and divide would be the other missing items.

One thing all four of these share is that we need to check that they are comparable. Your current check that the frames are close is a start, but I think it would be better to have a helper method to check that the frame types are the same, and that the frame and time arrays are not only close, but actually equal. I don't see how two waveforms would get similar but not equal frames or times.

It looks like these algebraic functions work currently, but just because of the inheritance from np.array, which should be considered a bug; when they involve two WaveformModes, they should always be checked for comparable data.

[akhairna]

Hi @moble, I am sorry I had to rebase it on top of the recent commits that were merged on upstream. Otherwise the git history wasn't clean.

[akhairna]

Hi @moble, @duetosymmetry, @keefemitman - I incorporated some of the suggestions.

• Added some documentation to the __mul__ method describing the order in which truncator is used, and its default if missing.
• Addition of method to check compatibility of WModes before multiplication.

Any suggestions on the updated structure of the __mul__ method and the helper function? I will look into the other algebraic operations too.
Q: After multiplication, I have put the multiplication_truncator attribute of the result to be max. Do you all think it should be sum instead?

[duetosymmetry]

Just so we don't forget: this PR is dependent on moble/spherical#28 . After that is merged and spherical has a version bump, the dependencies in pyproject.toml will need to be bumped in this PR.

[duetosymmetry]

New dep is spherical>=1.1.0

[akhairna]

Hi @moble, @keefemitman, and @duetosymmetry.
Here is an issue of shape of the data array that I am facing. Suppose I want to check this bondi constaint:
\dot{ψ₀} = ðψ₁ + 3 σ ψ₂
σ ψ₂ - I have taken the abs((σ ψ₂).spin_weight) of this product to be the l_min for slicing the data array.
ðψ₁ - Raises the spin but doesn't slice or discard the l=1 modes.
Operations like these result in mismatch between the shape of the data array which gives an error from __add__ method. What is a reasonable approach to deal with this?

1. Set the l_min to be 0 for all (which we were doing in scri).
2. Allow operations like eth and ethbar to change the shape of the data array (reasonable according to me).
3. Any other way to handle this?
   Please leave any feedback on the existing commits.

[akhairna]

@moble, @duetosymmetry & @keefemitman - Based on the discussion during vacuum call, I have modified the __add__ and __sub__ method. All the algebraic operations are working based on a few checks that I performed. Let me know if you have any feedback.

[keefemitman]

Thanks for doing this @akhairna. Unfortunately I think there's a few annoying bugs/issues present still.

[akhairna]

Hi @moble, @keefemitman, and @duetosymmetry. With these new commits, there shouldn't be any issue regarding shape of two WModes while doing algebra. As long as they are compatible via the is_compatible function, they can be added, subtracted or multiplied even if they differ in time_axis.
Please review it, and let me know if it looks alright.

[keefemitman]

Sorry to request another change @akhairna. But I approve the PR after this.