[PIPE2D-751] Please add a vectorised version of DetectorMap.getWavelength(fid, y) Created: 03/Mar/21  Updated: 06/Apr/21  Resolved: 06/Apr/21

Status: Won't Fix
Project: DRP 2-D Pipeline
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Story Priority: Normal
Reporter: rhl Assignee: price
Resolution: Won't Fix Votes: 0
Labels: SuNSS
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PIPE2D-805 Provide consistent API and documentat... Open
Sprint: 2DDRP-2021 A3, 2DDRP-2021 A 4

 Description   

The need to loop over y in python calling DetectorMap.getWavelength(fid, y) is a significant slow-down in SuNSS analysis. Please provide a version that accepts a numpy array as the second argument.



 Comments   
Comment by rhl [ 03/Mar/21 ]

The lack of this function is for example significantly slows the analysis of OH arcLines files from SuNSS.

Comment by price [ 04/Mar/21 ]

This function exists, but is named findWavelength, probably for historical reasons. So I'll take this as a request to clean up the interface in regard to get* vs find*. I propose:
1. Move findWavelength to getWavelength.
2. Move findPoint to getPoint.
3. Move findFiberId to getFiberId.

I'm a little concerned that get* implies a fast return without much in the way of calculation, which is not the case for any of these, but it's certainly the name we look for first, so perhaps convenience trumps that?

Comment by rhl [ 04/Mar/21 ]

Oh, good. I think the name's OK; the user doesn't know that we don't have these things pre-computed to logically it's just a `get`

Comment by rhl [ 04/Mar/21 ]

I don't see the corresponding vectorised function to return the fibre position (in x) at a set of wavelengths for a given fibre. This could be done by adding a vector overload to findPoint (returning a 2xn array), or a new function (getTracePosition?)

Comment by price [ 05/Mar/21 ]

There is a vectorised version of findPoint already. If it's not working for you, check that your types are correct (fiberId arrays should be int32, wavelength arrays should be float64) and the arrays are contiguous.

Comment by rhl [ 05/Mar/21 ]

Oh, that's great. I did look at the .h file, but didn't realise that you can vectorise over both fiberId, lambda which is exactly what I needed.

Comment by price [ 06/Mar/21 ]

OK, so what's the verdict on what work needs to be done?

Comment by rhl [ 10/Mar/21 ]

It sounds as if we should have a chat about a consistent API, but that all the needed functionality is present

Comment by hassan [ 03/Apr/21 ]

Can this ticket be closed, and a separate ticket on the API update filed?

Comment by hassan [ 06/Apr/21 ]

Problem is due to confusion over API. This is the subject of a separate ticket.

Generated at Sat Feb 10 15:57:18 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.