Skip to content

MOSAIC mIFU: tabular output - #968

Open
oczoske wants to merge 6 commits into
mainfrom
oc/mosaic_mifu
Open

MOSAIC mIFU: tabular output#968
oczoske wants to merge 6 commits into
mainfrom
oc/mosaic_mifu

Conversation

@oczoske

@oczoske oczoske commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

The PR introduces a new effect MosaicOutputFormat that sets the format produced by readout() for MOSAIC.

  • collapse1d : the spectra from the fibers in a bundle are summed to a single spectrum. To be used for the MOS modes.
  • table: the spectra from the fibers in a bundle are arranged in a binary fits table with one row per fiber. To be used for the mIFU modes.
  • image: the image format for the (pseudo-)detector is kept. Only for testing purposes.

@oczoske oczoske self-assigned this Aug 4, 2026
@oczoske oczoske added effects Related to a ScopeSim effect Science Team Scientific target audience spectroscopy Related to spectroscopic modes labels Aug 4, 2026
@oczoske oczoske moved this to 🏗️ In progress in ScopeSim for MOSAIC Aug 4, 2026
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.48718% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.90%. Comparing base (66081e6) to head (08b2358).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
scopesim/effects/mosaic_trace_list.py 79.48% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #968      +/-   ##
==========================================
+ Coverage   75.46%   75.90%   +0.43%     
==========================================
  Files          70       70              
  Lines        9003     9038      +35     
==========================================
+ Hits         6794     6860      +66     
+ Misses       2209     2178      -31     

☔ 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.

@teutoburg teutoburg moved this to 🏗 In progress in ScopeSim-development Aug 6, 2026
@oczoske
oczoske marked this pull request as ready for review August 6, 2026 17:19
@oczoske
oczoske requested review from astronomyk and teutoburg August 6, 2026 17:19
@oczoske oczoske moved this from 🏗 In progress to 👀 Awaiting Review in ScopeSim-development Aug 6, 2026

@teutoburg teutoburg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't try to fully understand the transformations behind this (it's too hot), I assume you did. Two minor comments, otherwise 👍

pass

else:
logger.warning("Unknown output format: %s", output_format)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.warning("Unknown output format: %s", output_format)
logger.error("Unknown output format: %s", output_format)

I'd say an unknown value here is more than a warning. logger.error only log the error, does not stop execution like raising an exception would, so the behavior is identical to setting output_format="image".

Comment on lines +289 to +302
i = 0
for sptid, spt in tqdm(self.spectral_traces.items(),
desc="Fiber traces", position=2):
x_mm = detwcs.all_pix2world(np.arange(image.shape[1]), 1, 0)[0]
lam = spt.x2lam(x_mm)

y_mm = spt.table["y"][0]
jfib = int(detwcs.all_world2pix(0, y_mm, 0)[1])
idarr.append(sptid)
xarr[i] = (self.aplist['left'][i] + self.aplist['right'][i]) / 2
yarr[i] = (self.aplist['top'][i] + self.aplist['bottom'][i]) / 2
specarr.append(image[jfib,])
lamarr.append(lam)
i += 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use enumerate instead of incrementing a counter?

@teutoburg teutoburg moved this from 🏗️ In progress to 👀 In review in ScopeSim for MOSAIC Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effects Related to a ScopeSim effect instrument-specific Science Team Scientific target audience spectroscopy Related to spectroscopic modes

Projects

Status: 👀 In review
Status: 👀 Awaiting Review

Development

Successfully merging this pull request may close these issues.

2 participants