Handle negative norm case where the result returns only 4 items

[Yong2Sheng]

I updated fast_ts_fit.py to handle different tuple lengths returned in result.

The code now checks the length of the result tuple to decide how to unpack it:

• If result has 5 items, it unpacks all of them.
• If result has 4 items, it unpacks the available values and sets the iteration to -1 to indicate a negative norm.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.05%. Comparing base (aecb327) to head (9566de5).

Files with missing lines	Patch %	Lines
cosipy/ts_map/fast_ts_fit.py	80.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
cosipy/ts_map/moc_ts_fit.py	100.00% <100.00%> (+77.66%)	⬆️
cosipy/ts_map/fast_ts_fit.py	90.37% <80.00%> (+0.75%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[israelmcmc]

Thanks @Yong2Sheng!

@NicoloParmiggiani kindly agreed to review this.

@Yong2Sheng: in the meantime, can you please add a unit test that executes the new lines? I think the easiest way would ne to take one of the existing tests, and subtract some counts to force negative. e.g.:
instead of

data_cds_array = FastTSMap.get_cds_array(src_bkg, [1,4]).flatten() ,
bkg_model_cds_array = FastTSMap.get_cds_array(bkg, [1,4]).flatten(),                                 

you can do:

data_cds_array = FastTSMap.get_cds_array(bkg, [1,4]).flatten() - 1,
bkg_model_cds_array = FastTSMap.get_cds_array(bkg, [1,4]).flatten(),                                 

There are other ways of course, whatever you prefer. I just want to avoid future changes resulting in this issue again.

[Yong2Sheng]

Hi @israelmcmc, thanks for the reminder! I will add the unit test for the new lines

[NicoloParmiggiani]

Hi @israelmcmc and @Yong2Sheng, I tested the code with the notebook that caused the original issue #380 and now it works properly.

[israelmcmc]

That's great, thanks @NicoloParmiggiani. When the coverage test is passing, you can go ahead and merge this.

[NicoloParmiggiani]

I tested the code with the notebook that previously caused the error, and it now works correctly.

[Yong2Sheng]

Hello @israelmcmc and @NicoloParmiggiani, I have added the unit test for the lines @israelmcmc requested. I also added/increased the unit test for test_fast_norm_fit.py (100%), test_fast_ts_map.py (90%), and test_moc_ts_fit.py (100%).

other changes

Bug fix one

I also fixed a bug when plotting the fitted multi-resolution map:

critical = MOCTSMap.get_chi_critical_value(containment = 0.9)

did not pass the containment parameter, so it always plot 90% confidence region.
Now it takes the correct containment parameter by the user.

critical = MOCTSMap.get_chi_critical_value(containment = containment)

Remove unnecessary checks

In function fill_up_moc_map, we check if the pixels are fitted more than once by

if idx.shape[0] != 1:
    raise ValueError(f"Pixel with pixel index {pixidx_} has {idx.shape[0]} fits! ")

However, we fit the pixels by orders in each iteration, so the check is redundant.

[Yong2Sheng]

Hi @NicoloParmiggiani, could you please take a look at the new changes? Thank you! Even though the PR is approved, it still looks like it needs to be merged manually.