[INFRA-93] Add new repository for coordinate transformation and its data Created: 07/Dec/16  Updated: 02/Oct/18  Resolved: 02/Oct/18

Status: Done
Project: Software Development Infrastructure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Story Priority: Major
Reporter: shimono Assignee: shimono
Resolution: Done Votes: 0
Labels: GitHub, MCS
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to INFRA-111 Add a section for development support... Done

 Description   

Aim of this repository are:

  • to provide base code modules (class/API) on conversion among various coordinates, both their functions (e.g. polynomial) and parameters
  • to store analysis results from simulation or modeling as reference

Also it is better to provide:

  • test data set and unit test (using test data set) of code modules for CI
  • jupyter notebook sample

This could be used from AG coordinate converter or fiber allocation software.

rhl I don't think we should merge this kind of items into datamodel, but be better to have separated. Is it ok for you on management point of view?
yuki.moritani Could you help once we will have this repo?
naoyuki.tamura Let us know if you have any comment/suggestion on project management aspect.
Kiyoto Yabe Just let you know on this.



 Comments   
Comment by yuki.moritani [ 02/Mar/17 ]

I'm more than happy to help it.
I think we should discuss what exactly I should push to them (format, etc.).

Could you make me sure.
You mean by fiber allocation software both ETS and FPS?
I mean, the repository includes not only sky-F3C transformation, but also MCS-F3C transformation?

Comment by shimono [ 16/May/17 ]

From recent discussions and updates, we'd make this happen.

  • repository owner: yuki.moritani
  • repository name: pfs_coordinates

This shall include

  • documents on functions and parameters in standard docs/ directory
  • conversion code (APIs) which can be used from software modules in eups way
Comment by yuki.moritani [ 18/May/17 ]

Given that coordinate transformation will be used mainly by ETS and FPS, I would like to hear what Martin Reinecke and chyan think of it.
Is it OK to you two?

Considering that mathematical formats of coordinate transformation will be updated at the commissioning (and might be even in scientific use), it would be better to have one repository, which accommodates transportation functions.

I think we should discuss argument values and return values, once we agree to have this repository.

Comment by Martin Reinecke [ 18/May/17 ]

I would be extremely happy to have this functionality available in a central place!

What ETS needs is basically the transformation from target RA/DEC to x/y PFS coordinates (for a given telescope pointing, position angle and observation time).

If I can help in any way, please let me know!

Comment by rhl [ 20/May/17 ]

Let us not reinvent the wheel. There is a standard description of the camera distortions in the HSC camera description, and we could probably reuse the format. It is about to be upgraded to support layers of distortions (e.g. camera optics, detectors, atmosphere), and this layering may well be useful for the MCS.

Comment by yuki.moritani [ 20/May/17 ]

> Martin Reinecke
Well, I think we should determine which should deal with telescope position, position angle, atmosphere etc.
Does current ver. of ETS assume these things? If I remember correctly, ETS changes position angle, but I'm not sure about other points.

> rhl
If we could reuse or refer the HSC format, it would be nice.
PFS optics is not the same as HSC optics (e.g. PFS has thicker glass than HSC's dewar window and + filters.
Also, PFS should determine it for entire region, whereas HSC possibly determines distortion detector by detector,
Considering these, PFS could use some of layers.

Comment by Martin Reinecke [ 22/May/17 ]

Currently, ETS takes RA/DEC of targets as input, which means that it (at least at the moment) has to deal with the full transformation machinery. Also, it is expected that ETS will try not only different position angles, but also slightly different telescope pointings in order to find good fiber assignments.

The current transformation from RA/DEC to x/y in ETS is not perfect; especially the distortions due to telescope optics are only very rough approximations.

Comment by shimono [ 31/May/17 ]

rhl it is quite nice if possible and was actually in some plan, to rely on the code of the HSC camera description. we are just at the starting point of studies on our distortion models among coordinates, and also we just started from fomulas of HSC with parameters from our calculation/simulation. As yuki.moritani pointed, we plan to include "ALL" into this model, so we may want to have more parameters, and also could need some special handling of calculation, such as to deal with differential changes by so called 'mod 60 degree' which is handled by Kiyoto Yabe and yuki.moritani, or AG calculation.

Comment by chihyi [ 08/Jun/17 ]

If F3C-MCSC transformation is included in this repository, then you need to take care of the distortion from the small telescope in front of metrology camera. IAA could provide such information.

Comment by yuki.moritani [ 08/Jun/17 ]

Well, when the spot on MCS from PFI was calculated, both the optics on MCS (e.g. primary mirror, flattner etc.) and PFI (e.g. WFC) are included.
The Subaru engineer used ZEMAX file provided from ASIAA.

Comment by chihyi [ 08/Jun/17 ]

Sorry that I don't know you have the ZEMAX file. Jennifer has done some study on fiber identification. If there are multiple fibers locate in the overlapping regions, then there is no confidential way to identify fibers. That's why we need to take images while COBRA is moving. Also this enables us to check if a missing fiber is hidden behind the dots.

Comment by Martin Reinecke [ 13/Jun/17 ]

Just asking for my clearer understanding:

Can I expect that at some point in the future there will be a Python function I can call, passing target RA/DEC, time, telescope pointing, position angle (and maybe a few more technical parameters), which returns target x/y positions on the focal plane?

If this is the case, I would redesign the ETS package with this in mind.

Comment by yuki.moritani [ 13/Jun/17 ]

Ah .. a few latest conversations are getting out of the scope this ticket.
I'm really sorry...

Yes. I'm expecting this is the case.

chihyi
Putting aside which way you need and we can provide for F3C-MCSC conversion, do you agree that we prepare a repository?
And let's discuss transformation itself in another ticket (or ML).

Comment by shimono [ 27/Jun/17 ]

ok, let me finish this specific ticket.

following https://github.com/Subaru-PFS/doc/blob/master/development-management/request.rst#add-new-github-repository

  • Name of new repository: pfs_coordinates
  • Short description of new repository: PFS Coordinate definitions and conversion codes
  • Responsible institution: IPMU
  • Corresponding JIRA project and component: INSTRM (or new project?), create new component with repository name
  • License: GPLv2
Comment by rhl [ 28/Jun/17 ]

I don't think that this is a good idea; you almost always regret mixing code and data (as von Neumann pointed out); we need to split the code from the data (which will need to be versioned; we already have a way of doing this for e.g. flat fields and I think we should use the same mechanism).

Who owns the schema for these files – they need to be in the data model. As discussed above there are ways of composing transforms in the PFS codebase (based on starlink's AST library). Are these sufficient?

What are the proposed APIs for all these transformations?

Why not a BSD license? In general GPL turns out to be a mistake, so why continue making it?

Comment by Martin Reinecke [ 28/Jun/17 ]

I agree that mixing code and data is usually not a good idea; but I don't see the problem with code and data sitting alongside each other in the same repository.

Why not a BSD license? In general GPL turns out to be a mistake, so why continue making it?

I don't agree with your assertion about GPL.

But independent of that: if GPL indeed turns out to be a mistake, it is trivial to re-license the code in question to BSD.

Going from BSD to GPL (for released code) is impossible, however. You can't revoke rights that you have granted before.

Comment by rhl [ 28/Jun/17 ]

The problem is that you need to version the data independently of the code.

Comment by Martin Reinecke [ 28/Jun/17 ]

I see your point.

Assuming that both code and data quality strictly improve over time, a joint version number could work, but that's not the world we live in

I'm not sure how separate two entities have to be in git in oder to tag them independently, but I agree that this should be possible.

Comment by shimono [ 28/Jun/17 ]

> I don't think that this is a good idea; you almost always regret mixing code and data (as von Neumann pointed out); we need to split the code from the data (which will need to be versioned; we already have a way of doing this for e.g. flat fields and I think we should use the same mechanism).
Really mourning to be said as so - always regret mixing.
I REALLY trying to getting things to be tracked and separate data/code/config etc. to be well tracked, within this project. I REALLY FRUSTRATED MANY PEOPLE EVEN IN THE PROJECT OFFICE DOES NOT TRY TO USE TICKETS NOR REPOSITORIES, OR EVEN THEY HESITATE TO USE THINGS!
In this ticket, at least, I want to gain things to be tracked, that's already a difficult issue.
If I want to go lovely world, and I can propose anything, I would want to have

  • formulas and data formats with well described documents into datamodel
  • transformation code to one repository
  • parameters of transformation in an another repository
    I think datamodel itself is something to be considered as definition (or coded reference), and configuration/parameter shall be split from it.

> Who owns the schema for these files – they need to be in the data model. As discussed above there are ways of composing transforms in the PFS codebase (based on starlink's AST library). Are these sufficient?
I agree this. I just thought about parameters and codes for transformation.

> What are the proposed APIs for all these transformations?
Shall be by yuki.moritani

> Why not a BSD license? In general GPL turns out to be a mistake, so why continue making it?
I actually love MIT or BSD (which are used for some pfs_* repos). GPL is just follow princeton's GPLv3, but I believe v2 is better for our life than v3.
If we all agree on moving from GPL to another, it is possible at anytime. of course older version need to remain, although.

Comment by hassan [ 18/May/18 ]

The need for at least one independent repository for PFS data for development and test purposes was highlighted during the recent PFI meeting in Taipei 14-15 May. I plan to look into options over the coming months and will discuss options already considered with rhll and shimono and will raise a separate ticket if we find the need for that.

Comment by cloomis [ 08/Jun/18 ]

As far as the code repo goes, pfs_utils was created for modules which 1) we expect will be used by disparate parts of the project (ICS, DRP, ETS) and 2) are unlikely to depend on many other packages. Currently the only module is fiberids, which contains the (static) fiber mapping (center PFI coordinates, slit positions, module number, etc).

I suggest that this would be a reasonable place to put distortion code. And at least for now perhaps the data files.

Comment by cloomis [ 08/Jun/18 ]

And if this is deemed reasonable, I'd be glad to move the existing code over from ics_fpsActor and neaten it up.

Comment by yuki.moritani [ 08/Jun/18 ]

Actually, since I sent the code to Chi-Hung, I've modified the code to merge with sky-PFI transformation (not finished yet, though). So if possible, I'd like to upload the new one.

Comment by yuki.moritani [ 08/Jun/18 ]

I think I can replace after Craig's work, although I'm not sure which is easier to us.

Comment by cloomis [ 11/Jul/18 ]

yuki.moritani Is this just a new version of ics_fpsActor's CoordTransp.py plus similar files? If so, I'll suggest that you update that now and move it later.

(Martin Reinecke, this is basically MCSxy_to_PFIxy(zenithAngle), plus whatever new sky_to_PFI() routines have been added.

Comment by yuki.moritani [ 12/Jul/18 ]

I use the same routines for all kind of coordinate transformations, just changing coefficients via dictionary.

So I personally don't think it suitable to update one to ics_fps Actor, actually..

For the same reason, I think . only MCS label isnot enough > hassan

Comment by cloomis [ 12/Jul/18 ]

OK. Please add to pfs_utils, maybe in pfs/utils/transforms/ or pfs/utils/coordinates/?

Comment by hassan [ 12/Jul/18 ]

yuki.moritani agree that MCS label is not enough. I added that as this issue is relevant for MCS activities, but doesn't mean it is not relevant for others also.

In addition as I was discussing with Craig earlier today, this particular issue should probably be resolved based on the GitHub repo creation alone, and separate tickets should be raised on the activities that have been discussed in the comments here.

Comment by yuki.moritani [ 13/Jul/18 ]

cloomis , All right. maybe the latter?? Anyway I'll push my current code next week.

hassan , thank you for explanation... for (my) future reference, could you kindly tell where I can see all available labels?
And I agree, the original goal of this ticket is indeed to make a place for coordinate transformation I'm afraid...

Comment by hassan [ 18/Jul/18 ]

Sorry yuki.moritani for not responding to your question earlier. There is no obvious means of listing all the available labels. One option is to install the labels gadget (https://confluence.atlassian.com/jiracoreserver073/gadgets-for-jira-applications-861257078.html?_ga=2.22050875.1837736392.1531921220-800630600.1525207411), but all I do is to search for issues by label and a pop-up appears of all the labels.

At this point the labels that currently exist are FITS, GitHub, MCS and SM1.

This thread is a little out of context for this issue, so I will also copy my response to a slack channel so that this side-discussion is preserved and is easy to access.

Comment by yuki.moritani [ 17/Aug/18 ]

cloomis I have uploaded the current code
https://github.com/Subaru-PFS/pfs_utils/tree/distortion_201808ym

At ICS teeconf. today, you told you think separating coefficients and code itself.
Could you please tell how to proceed with the codes?
By the way, the code has local path ... it should be modified at least.

Comment by hassan [ 02/Oct/18 ]

From discussions with @cloomis, this ticket is done with the actions of yuki.moritani.

Comment by yuki.moritani [ 02/Oct/18 ]

Well I'm sorry ... I have not merged the brunch yet... so I've done now.

(Just for recording.)

Generated at Sat Feb 10 16:49:21 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.