Implementation available at branch tickets/FIBERALLOC-35.

For reviewers: the changes look much more dramatic than they actually are. Most changes are just removal of files that are no longer needed. The actual code changes to netflow itself are a few lines.

I just tried it but could not get it to run. I think the first thing that we need to do is to update the list of requirements. 

I git clones pfs_utils and symlinked the python/pfs subdirectory into ets_fiberalloc, but this is probably not the canonical way of doing things?

After installing astroplan I get

mxhf@mxhf-macbook-93:~/work/MPE/pfs/src/ets_fiberalloc$ python demo_netflow.py

WARNING: failed to download http://maia.usno.navy.mil/ser7/finals2000A.all, using local IERS-B: <urlopen error [Errno 8] nodename nor servname provided, or not known. requested URL: http://maia.usno.navy.mil/ser7/finals2000A.all> [astropy.utils.iers.iers]Traceback (most recent call last):  File "demo_netflow.py", line 51, in <module>    tpos = [tele.get_fp_positions(tgt) for tele in telescopes]  File "demo_netflow.py", line 51, in <listcomp>    tpos = [tele.get_fp_positions(tgt) for tele in telescopes]  File "/Users/mxhf/ownCloudRZG/work/MPE/pfs/src/ets_fiberalloc/ets_fiber_assigner/netflow.py", line 451, in get_fp_positions    cent=np.array([self._ra, self._dec]), time=self._time)  File "/Users/mxhf/ownCloudRZG/work/MPE/pfs/src/ets_fiberalloc/pfs/utils/coordinates/CoordTransp.py", line 68, in CoordinateTransform    xyin, inr, za1 = convert_in_position(xyin, za, inr, pa, c, cent, time)  File "/Users/mxhf/ownCloudRZG/work/MPE/pfs/src/ets_fiberalloc/pfs/utils/coordinates/CoordTransp.py", line 250, in convert_in_position    za = za[0]TypeError: 'float' object is not subscriptable

It looks like it is trying to download information somewhere.

The installation problems with the pfs_utils package are known and will be hopefully resolved soon (see https://pfspipe.ipmu.jp/jira/browse/INSTRM-1047 ). For the moment, the cloning and symlinking approach should work OK, but of course this is not a permanent solution.

Concerning the failed download I have no idea what's happening. I remember seeing some on-the-fly downloads by astropy when I ran this first, but at the moment I don't observe them ... probably my local data are recent enough.

Still, downloading files from an US navy server feels a bit ... unexpected.

The last error in your log is actually unrelated to the failed downloads. Moritani-san and I tried to fix this some time ago, but the fix doesn't seem to be complete.

Could you please try to replace line 251 of pfs_utils/python/pfs/utils/coordinates/CoordTransp.py from

except IndexError:

to

except (IndexError, TypeError):

and see whether that fixes the problem?

I think this is ephemeridial information. Do you think you could identify the files that got downloaded on your side?

I can try, but failure to download these files should not result in an error that stops the code. The crash you see is unrelated to these files.

ok, with the hack to pfs_utils it seems to work. The collision simulation dip not show up (the window open). Is this working for you?

Yes, I see the images of the focal plane, and the animations of the collisions.

OK, I do seem to get those if I start the script from within ipython.

Does the netflow code now have the option to add a constraint on pairs of colliding fibers and to reoptimize?

I get the plots also when running with standard python3.

 Does the netflow code now have the option to add a constraint on pairs of colliding fibers and to reoptimize?

It has the option of recomputing parts of an already solved problem. This could be "mis-used" for this purpose, for example by fixing one cobra of the colliding pair to the assigned target, removing the target of the other one from the target list and re-running. I'm not sure if this is a good way of doing things however.

To improve on this, I need some guidance on how to add the "constraint on pairs of colliding fibers". If we assume that cobra A observing target x and cobra B observing target y leads to a collision, I can add the constraint that at most one of "A observes x" and "B observes y" is true. But this can lead to an iteration which takes forever to complete. Also, do we reoptimize from scratch, and if not, which Cobra-target assignments do we keep?
 

The exact solution is given in the jupyter notebook that I gave you some time ago. But the theory is quite simple. you already avoid endpoint and elbow collisions. You do so by adding a constraint like flow_through(object_A) + flow_through(object_B) <= 1 for any pair of objects that are too close to each other to be observed simultaneously (see netflow.py line 340 ff).

For trajectory collisions we do exactly the same. So for any pair of object where we detect a collision in Javier's code, we add such a constraint equation.

 Then we reoptimize. Note that we never drop targets!

Which part of the problem to reoptimize is an interesting question. I had run some experiments, where I fixed all flows outside of a certain distance of a colliding pair. The distance was a parameter. This is certainly an easy way.... 

Does this make sense?

I think that your suggestion is equivalent to my second one. It should work "in principle"; I'm just quite worried that we can make no useful statement at all about how many iterations will be needed before we reach a solution (or have to give up). Given that we only add constraints at a rate of one per colliding pair per iteration, this could take a very long time.

Concerning the amount of fibers that need to be re-run: this probably also depends quite strongly on which kind of targets are observed by the colliding cobras. Assuming, for example, that both cobras should have observed calibration targets, we need to make sure that in the re-optimized region there is at least one "spare" calibration target, and (most likely) that its observing cobra lies on the appropriate section of the spectrograph slit.

Or, assuming that both observed targets are science targets requiring more than one visit... in this case we have to re-optimize not just one visit, but all. This is a pretty big can of worrms.

I understand your concerns. This is why we have tested this approach now years ago and found this approach to work. The reason probably being that trajectory collisions are so rare in the first place. Nevertheless, we do have to absolutely avoid them.

I'm fairly sure we didn't have a chance to test the complications I mentioned, among other reasons because I don't think we have the necessary constraints for the calibration/slit mappings even now. So I thought a word of caution would be appropriate

I will update the assigner as discussed, excluding all combinations which the cobraOps package marks as trajectory collisions.

If we have to avoid collisions at all costs, how do we deal with the stochastic nature of the cobra movements and the inexact nature of the motor maps? Do we add a safety margin (I think we should!), and how much?

Let' s continue this discussion at FIBERALLOC-38!

Should be implemented on tickets/FIBERALLOC-35.

I'm not sure who should officially review this...

Martin Reinecke: commit ac1f87a has title 'switch to coordinate conversion routines from pfs_utils, part 1'. I can't see a 'part 2' commit. Is that intentional?

Sorry, that was sloppiness on my part. After the first commit I thought it might be better to actually use descriptive commit messages instead of simply counting commits until the goal was reached.

One fairly unrelated question that occurred to me when I was looking at the commits again: currently we are using the "ets" prefix in our package names with an underscore, i.e. "ets_fiber_assigner", "ets_shuffle"... but most other packages actually use this as a "master package" name, like "pfs.utils.*". Should we change this? If yes, then perhaps the earlier, the better. Of course, I'd open new issues for this.

The changes look fine to me. Feel free to merge to master.

Regarding the question for package naming: I'm looking into this with Project Office. It makes sense to rename, but may not be worth the effort. I'll get back to you on that separately next week.

Thank you very much for reviewing!

I think if the ticket branch has been merged, the ticket can be closed.