[DAMD-108] Please add spectrograph to pfsDesign/pfsConfig file Created: 03/Mar/21 Updated: 12/Mar/21 Resolved: 12/Mar/21 |
|
| Status: | Done |
| Project: | Data Model |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Story | Priority: | Normal |
| Reporter: | rhl | Assignee: | price |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | 2DDRP-2021 A3 |
| Reviewers: | rhl |
| Description |
|
When analysing SuNSS data it's very convenient for the pfsArm and pfsConfig files to line up, each with the same number of fibres. This won't be true for the full PFS system with four spectrographs, but if we add a spectrograph field to the pfsConfig it'll be easy to work with the data. So let's do it before SM2 arrives. |
| Comments |
| Comment by price [ 04/Mar/21 ] |
|
What should the value be in each of the following cases? |
| Comment by price [ 09/Mar/21 ] |
|
Cancel my previous question. I thought this was a request for a header telling which spectrographs were expected to be used for a particular observation. Now I understand this to be a request to easily provide the subset of fibers for a particular spectrograph. I propose to add selectBySpectrograph, along the same lines of selectByTargetType, which will return the indices of all the fibers on that spectrograph. |
| Comment by price [ 11/Mar/21 ] |
|
I've pushed an implementation that adds spectrograph (and fiberHole) properties to PfsConfig and PfsArm. rhl, does this do what you need? |
| Comment by price [ 11/Mar/21 ] |
|
rhl rightly points out that the magic number (651) and the functions to translate fiberId to spectrograph and fiberHole belong in pfs_utils, but I'm concerned about adding a dependency to the datamodel package. I'm happy to do either, but someone needs to decide which option we go with. |
| Comment by hassan [ 11/Mar/21 ] |
|
What are the disadvantages of having datamodel dependent on pfs_utils ? I also felt that the functions should reside under pfs_utils. |
| Comment by price [ 11/Mar/21 ] |
|
rhl points out that we could put the functions in pfs_utils, and put the new property methods in subclasses in drp_stella. This should make everyone happy. |
| Comment by hassan [ 11/Mar/21 ] |
|
I was thinking the same as an alternative option. Fine by me also! |
| Comment by price [ 11/Mar/21 ] |
|
Done. Deleted the branch in datamodel, and there are now ticket branches in pfs_utils and drp_stella. I also fixed some off-by-one bugs (all these identifiers are Fortrash!) and added tests. |
| Comment by rhl [ 12/Mar/21 ] |
|
How does this relate to |
| Comment by price [ 12/Mar/21 ] |
|
It doesn't. I'm working on that independently in the background. |
| Comment by rhl [ 12/Mar/21 ] |
|
Yes, you're right. Sorry. |
| Comment by rhl [ 12/Mar/21 ] |
|
Looks OK. Blame JEG for the 1-based names, I'm not sure why he did that but it's far far too late to change. |
| Comment by price [ 12/Mar/21 ] |
|
Merged to master. |