Issues with adding missing orders

[kbwestfall]

This PR does two things:

• Application to some HIRES datasets highlight that the left-right syncing procedure performed after adding missing orders has some conceptual issues. This introduces a short-cut that we'll need to test. Instead of redoing the full syncing procedure, the code now only checks that the edges are still synced after the missed orders are added.
• When adding missed orders, the code will nominally add orders regardless of how much they overlap. This adds a new parameters max_overlap that can be used to limit the added orders to only those that have less than this maximum overlap with adjacent orders.

There are also some random doc fixes. I will run and post the dev-suite results.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 14.61187% with 187 lines in your changes missing coverage. Please review.

Project coverage is 38.03%. Comparing base (2b53647) to head (fe64515).

Files with missing lines	Patch %	Lines
pypeit/edgetrace.py	4.16%	138 Missing ⚠️
pypeit/spec2dobj.py	5.26%	18 Missing ⚠️
pypeit/specobjs.py	7.14%	13 Missing ⚠️
pypeit/scripts/chk_flexure.py	0.00%	12 Missing ⚠️
pypeit/par/pypeitpar.py	84.61%	2 Missing ⚠️
pypeit/wavecalib.py	60.00%	2 Missing ⚠️
pypeit/core/trace.py	90.00%	1 Missing ⚠️
pypeit/scripts/trace_edges.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1835      +/-   ##
===========================================
- Coverage    38.07%   38.03%   -0.05%     
===========================================
  Files          211      211              
  Lines        49051    49160     +109     
===========================================
+ Hits         18675    18696      +21     
- Misses       30376    30464      +88     

Flag	Coverage Δ
	?

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

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

[kbwestfall]

The results of the run from a couple of weeks ago:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  256 passed, 6736 warnings in 500.40s (0:08:20) ---
--- PYTEST UNIT TESTS PASSED  148 passed, 2662 warnings in 936.81s (0:15:36) ---
--- PYTEST VET TESTS FAILED  2 failed, 60 passed, 98674 warnings in 5529.67s (1:32:09) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 2/239 TESTS  ---
Failed tests:
    bok_bc/600 pypeit
    keck_hires/J0306+1853_U074_RED_C2_ECH_0.72_XD_1.42_1x3 pypeit
Skipped tests:
Testing Started at 2024-07-31T18:15:57.941519
Testing Completed at 2024-08-01T12:14:56.105569
Total Time: 17:58:58.164050

Bok_bc/600 is now irrelevant.

I'm going to look into the HIRES failure today.

[debora-pe]

Hey @kbwestfall. I added some edit suggestions that fix the problem with the dataset that I shared with you on Slack, but I need you to check it to make sure I understood the code.

I also notice that the parameter order_outlier is not helping much. In a couple of datasets makes things worse. In one case makes the reduction to fail and in another adds a missing order (I'm guessing one that was considered outlier) in a wrong location and fail later on.

There are still 2 more failures that I am trying to debug. They happen after the edge tracing (wavecalib and flat fielding) but I'm not sure they are caused by edge tracing or by recent changes to the code from other PRs.

[kbwestfall]

I added some edit suggestions that fix the problem

Looks good! Thanks for finding the fix, and sorry again for not catching these bugs.

I also notice that the parameter order_outlier is not helping much. In a couple of datasets makes things worse. In one case makes the reduction to fail and in another adds a missing order (I'm guessing one that was considered outlier) in a wrong location and fail later on.

We can try setting the default back to None for this. I tested doing this against the dataset that was causing the original problem and it works fine. I.e., the other changes I made to the code catch the underlying issue for this dataset, so we may not need the order_outlier parameter in general.

[debora-pe]

@kbwestfall I think we should just run the tests and then we can merge. There is one last favor...Can you please add some documentation about the the QA plot for the missing orders. With all the new improvements, it would be very useful to have something written that explain what all the different points in the plot are.

Thank you so much for helping with this.

[badpandabear]

Result of Oct 7 dev-suite run:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  256 passed, 6840 warnings in 609.23s (0:10:09) ---
--- PYTEST UNIT TESTS PASSED  152 passed, 2670 warnings in 1092.98s (0:18:12) ---
--- PYTEST VET TESTS PASSED  62 passed, 106525 warnings in 6059.94s (1:40:59) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 247/247 TESTS  ---
Testing Started at 2024-10-07T22:02:52.633780
Testing Completed at 2024-10-08T17:54:37.289253