I'd be very happy to merge the changes! Is it possible to have the modified version committed to a new branch on https://github.com/Subaru-PFS/ets_fiberalloc, so that we can look at the differences easily?

OK. monodera, could you update subaru_fiber_allocation_2021nov.py (if needed) and push to a new branch of ets_fiberalloc? 

I have pushed a new branch (tickets/OBSPROC-21/monodera) to the repository.

https://github.com/Subaru-PFS/ets_fiberalloc/tree/tickets/OBSPROC-21/monodera

Could you please check if the pushed script and readme file is okay?

Thank you very much for providing the script and documentation! I think this is a great impovement over `commissioning.py` (which was originally meant for demonstration purposes only!) and should be preferred to it in the future.

Related to this I have one technical question, perhaps mainly to hassan: should we continue to host this file inside the fiber allocation package? I think it is of very broad interest and probably deserves a separate repository. I'm of course very happy to keep it in ets_fiberalloc, but not many people will expect it in there.

Hi Martin, I agree that commissioning.py probably should be in a separate repository, although it's not obvious at this moment which one. I will discuss this with yuki.moritani. In the meantime, let's keep it in ets_fiberalloc.

Wanqiu He is developing the pointing optimizer and there is a part to allocate fibers by calling netflow. I think it would be great to make a repository to integrate them and work together.

monodera I have put a simplified version of PPP to the repository:
https://github.com/Subaru-PFS/ets_pointing/tree/wanqqq-test/PPP

The "netflow" part of the script may be the one you want to check.

Regarding Hasan's comment on repository

Hi Martin, I agree that commissioning.py probably should be in a separate repository, although it's not obvious at this moment which one. I will discuss this with yuki.moritani. In the meantime, let's keep it in ets_fiberalloc.

Hassan and I talked about it. We think it nicer to have commissioning.py on{{ ets_pointing}} repository, rather than ets_fiberalloc ...Also it is nice to rename .py to more proper one. What o you think?

I'm updating a script to generate pfsDesign files. I'm trying to split functions so that we can port it into the ets_pointing repository. The code can be found in the commissioning_2022may branch in the ets_target_database repository.

https://github.com/Subaru-PFS/ets_target_database/tree/commissioning_2022may

Note that the README is not updated (but should be more or less similar) and will be done by the commissioning run.

In the previous commissioning.py script, there is a getBench function. I'm wondering if we still need it or not. In the script provided by Wanqiu He, it uses bench = Bench(layout="full") with no additional adjustment.

This is a great change! The goal was to remove all the individual fixes in getBench() over time, and if this is indeed now possible, I'm very happy.

Did you get some error before? I haven't tried to remove getBench() at all, so I don't know if it works properly or not. Also, cobraCoach may be anyway needed for the collision check?

Yes, there were errors before, e.g. "negative link length detected".

Sorry, I think that were actually link lengths of zero, or impossibly long links.

Thanks. Then, it looks promising.

While making the adjustments necessary for INSTRM-1582 I wondered whether we need to take proper motion and parallax into account for the fiber targets. So far we only use PM information when determining the exact position of the guide star candidates.
If we need this for the science targets, I expect we have to enhance the current target file format by a few extra columns.
What do you think, monodera?

I think this is an excellent idea.

The attached file has a function

"update_coords_for_proper_motion"

that may be helpflul.

do_shuffle_target.py

We have this code already in the "ets_shuffle.convenience" module. But if I understand correctly, this task will actually be done for us by the updated coordinate transforms in "pfs_utils", so we can probably remove it. Still, we have to tell "pfs_utils" the proper motion of our targets, which we currently don't know (since it is not listed in the target files).

As it is natural to include proper motions for Galactic science as well as flux standard stars, it would be nice to have them in the table. I guess just adding extra columns shouldn't be a problem. I implemented pm info in the targetDB with the default values of zero for (pmra, pmdec, and parallax).

Slightly off topic, but related: In the Gaia catalogs, epoch is defined as a float (2015.5 for DR2), while epoch in the PFS datamodel is defined as a 7-character string like "J2015.5". The conversion is straightforward, but looks redundant as users convert floats to strings at Phase 2, the observatory store them as strings in the DB, and the codes convert strings to floats / floats to strings (perhaps multiple times).

Maybe targetDB (and upstream DBs) can use epoch in float (only accepts Julian epochs) and convert it to strings to conform the datamodel when writing to the pfsDesign.

BTW, a more comprehensive example of proper motion (+distance) correction can be found here, though it does not seem to be relevant for us.

https://docs.astropy.org/en/stable/coordinates/apply_space_motion.html

I'm wondering whether proper motions need to be corrected before fiber assignment in order to take dots and fiber collisions into account. If pfs_utils takes care of it before writing to pfsDesign after fiber assignment, should input coordinates be the original ones before the correction? I think as long as the epoch is set and sent properly to pfs_utils, the correction can be before the fiber assignment. In any case, if there is a single function in the pfs_utils for the pm correction would be nice to avoid unnecessary redundancy.

hassan pfsDesign-0x66252be36b4c117a.fits is an example from the current code. There are many unassigned fibers mainly due to the lack of enough science targets.

I used the following parameters.
ra = 215.0
dec = 43.0
pa = 0.0
obstime = 2022-05-20T15:00:00Z

Thank you. I'll do a double check.

Thanks. I will also have a look.

I'm wondering whether proper motions need to be corrected before fiber assignment in order to take dots and fiber collisions into account. If pfs_utils takes care of it before writing to pfsDesign after fiber assignment, should input coordinates be the original ones before the correction? I think as long as the epoch is set and sent properly to pfs_utils, the correction can be before the fiber assignment. In any case, if there is a single function in the pfs_utils for the pm correction would be nice to avoid unnecessary redundancy.

My understanding is the following:

• up to now, if I wanted the exact position of a target in the focal plane, I had to do all necessary corrections beforehand and then call the coordinate transform from pfs.utils on the resulting RA/Dec.
• in the future, I will simply pass the catalog RA/Dec, epoch, pm, parallax and observation time to the transform routine and get the correct focal plane locations back.

In other words I will never even see the corrected RA/Dec values (which is fine, since the fiber assigner only works with focal plane coordinates). That of course leaves the question which sky coordinates should be written into the pfs design file: the catalog ones (which could also be found in the catalog) or the corrected ones (which I don't have any more with the new approach).

I fear that my understanding of the pfs design file is quite insufficient and therefore some of the data in there feel redundant to me. If the object and catalog ID of a target are known, as well as the observation time, then RA/Dec can in principle be obtained on the fly.

From today's ICS/PFI telecon: need to do a tweak from final RA/DEC to final observed x/y. Design would have nominal RA/DEC. Config will have final RA/DEC taking into account airmass. The SHA for the Config should not be recalculated from the updated RA/DEC.

monodera Kiyoto Yabe: are we in a position to close this ticket now, given what is already implemented? We can create new tickets to capture additional work.

Yes, I guess we can merge `commissioning_2022may` branch and close this ticket. We can open `commissioning_2022jun` ticket if needed (probably we need).

I have just merged the commissioning2022may branch to the main in the ets_target_database repository. I agree to close the ticket now.

We will still need to discuss where the scripts to be hosted, but can be discussed in a separate ticket.

monodera: I think Yuki suggested ets_pointing as the git repo location for these scripts (https://pfspipe.ipmu.jp/jira/browse/OBSPROC-21?focusedCommentId=30794&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-30794). Or is your question related to something else? In any case, that can be captured in a separate ticket as you say.

I forgot about that comment. I agree with her.

I close this ticket as we have agreed and will open another ticket about the script location.