Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default MATAB argument #189

Closed
Skyhawk172 opened this issue Dec 16, 2024 · 3 comments · Fixed by #191
Closed

Default MATAB argument #189

Skyhawk172 opened this issue Dec 16, 2024 · 3 comments · Fixed by #191

Comments

@Skyhawk172
Copy link
Contributor

Skyhawk172 commented Dec 16, 2024

Using the romanisim-make-image script, the default ma_table_number is set to 1, but this value is not valid anymore since the implementation of the new hard-coded MA tables.

In ris_make_utils.set_metadata (and in other places as well), the default ma_table_number is set to 4. It looks like a quick fix is to update the default value of the parser in the romanisim-make-image script.

However, when I specify an ma_table_number of 4 (the default), then I get the following error:

CRDS - ERROR - Error determining best reference for 'dark' = parameter='ROMAN.META.EXPOSURE.MA_TABLE_NUMBER' value='17.0' is not in ['1.0', '101.0', '102.0', '103.0', '104.0', '105.0', '106.0', '107.0', '108.0', '109.0', '110.0']

@eunkyuh

@schlafly
Copy link
Collaborator

What CRDS server are you pointing to? I agree with you that the current romanisim-make-image default of 1 is wrong now, and the linked PR #191 should address that. It looks to me that with that change running something like:
romanisim-make-image out2.asdf --usecrds
runs without issue.

>>> asdf.open('out2.asdf')['roman']['meta']['exposure']['ma_table_number']
4

I'm confused in your report where the 17 is coming from, but I can't immediately reproduce it. It looks to me like roman-crds and roman-crds-test both have darks for MA table 4, so those seem fine?

@Skyhawk172
Copy link
Contributor Author

The 17 is just because we tried a few different values... but we knew they wouldn't work based on the code.

Also, thanks for pointing our the CRDS server... That reminded me that I'm using a specific outdated context. I'm now using the latest context (0065) and it's running now, as long as I override the default MATAB number.

So the only thing remaining minor thing to fix is the default ma_table_number when parsing the command line arguments.

@schlafly
Copy link
Collaborator

Great, yes, that's what #191 does. I'll go ahead and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants