nasa / dorado-scheduling

Dorado observation planning and scheduling simulations

Other

23 stars 8 forks source link

Minor cleanup in compute_overlap #46

Closed lpsinger closed 3 years ago

lpsinger commented 3 years ago

This removes Healpy from the script and uses the high-level astropy-healpix API instead.

I don't understand what the compute_overlap function does. It looks like it is computing the area of largest overlap between the first 100 tiles and the rest of the tiles. However, it is multiply the number of pixels in the overlap by the pixel linear resolution in arc minutes, which does not make any dimensional sense. The number of pixels in the overlap is a measure of area.

codecov[bot] commented 3 years ago

Codecov Report

Merging #46 (fa0cb64) into main (8078b5d) will increase coverage by 0.10%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   26.63%   26.74%   +0.10%     
==========================================
  Files          28       28              
  Lines        1359     1350       -9     
==========================================
- Hits          362      361       -1     
+ Misses        997      989       -8

Impacted Files	Coverage Δ
dorado/scheduling/scripts/simsurvey.py	`9.00% <0.00%> (-0.09%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8078b5d...fa0cb64. Read the comment docs.

mcoughlin commented 3 years ago

Looks like it is failing lint. This was just a quick cross-check for me, the whole function can be removed.

lpsinger commented 3 years ago

Looks like it is failing lint.

I think the lint errors should be fixed now.

This was just a quick cross-check for me, the whole function can be removed.

It can? It's not dead code, it's toggled by the --doOverlap flag.

mcoughlin commented 3 years ago

You are computing field overlap elsewhere no?

lpsinger commented 3 years ago

You are computing field overlap elsewhere no?

I'm not sure what you mean. The overlap is accounted for in the objective function, so I never directly calculate it. Do you need it for something?

mcoughlin commented 3 years ago

Basically if someone wants to check quickly what overlap their grid has, it is nice to be able to easily check.

lpsinger commented 3 years ago

Basically if someone wants to check quickly what overlap their grid has, it is nice to be able to easily check.

I see. Well, I don't want to push you to remove it, if it is useful, but it does currently have nonsensical units, which should be fixed.

mcoughlin commented 3 years ago

Sounds good to me.