desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

Coverage of sv1, sv2, sv3 has dropped dramatically #798

Closed weaverba137 closed 1 year ago

weaverba137 commented 1 year ago

As reported in #797, coverage dropped dramatically after merging that PR. The culprit is that desitarget.sv1, desitarget.sv2, and desitarget.sv3 tests are not being run or otherwise skipping that code. For example coverage of desitarget.sv1 dropped from about 85% to about 17%.

No other components of desitarget show a significant change in coverage.

weaverba137 commented 1 year ago

In fact, it's possible to be even more precise: the specific files desitarget/sv1/sv1_cuts.py, desitarget/sv2/sv2_cuts.py, desitarget/sv3/sv3_cuts.py are exactly where the coverage dropped.

geordie666 commented 1 year ago

In case it's informative, here are the results of running coverage at NERSC in my version of the repo (after pulling main). No such corresponding drops in desitarget.sv1.sv1_cuts.py, desitarget.sv2.sv2_cuts.py or desitarget.sv3.sv3_cuts.py have occurred:

pytest --cov
======================================================
platform linux -- Python 3.9.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: $HOME/git/desitarget
plugins: astropy-header-0.1.2, asdf-2.7.2, filter-subpackage-0.1.1, hypothesis-6.31.6, openfiles-0.5.0, remotedata-0.3.2, arraydiff-0.3, mock-3.6.1, cov-3.0.0, doctestplus-0.11.2
collected 105 items                                                                                                                                                                                        

py/desitarget/test/test_brick_fluctuation.py ss. 
py/desitarget/test/test_brightmask.py ........       
py/desitarget/test/test_cmx.py ........                   
py/desitarget/test/test_cuts.py ................           
py/desitarget/test/test_geomask.py ...                
py/desitarget/test/test_io.py .........                      
py/desitarget/test/test_mock_build.py s.s.         
py/desitarget/test/test_mtl.py s..........                 
py/desitarget/test/test_mtl_multiple.py ....          
py/desitarget/test/test_priorities.py ........            
py/desitarget/test/test_qa.py ......                         
py/desitarget/test/test_secondary.py ....              
py/desitarget/test/test_skyfibers.py .......             
py/desitarget/test/test_subpriority.py ...               
py/desitarget/test/test_sv.py ..                              
py/desitarget/test/test_targetids.py ...                  
py/desitarget/test/test_top_level.py ..    
py/desitarget/test/test_units.py ....                                

----------- coverage: platform linux, python 3.9.7-final-0 -----------
Name                                  Stmts   Miss  Cover
---------------------------------------------------------
py/desitarget/QA.py                    1011    146    86%
py/desitarget/ToO.py                    180    180     0%
py/desitarget/__init__.py                 3      0   100%
py/desitarget/brightmask.py             365    103    72%
py/desitarget/cmx/__init__.py             0      0   100%
py/desitarget/cmx/cmx_cuts.py           995    143    86%
py/desitarget/cmx/cmx_targetmask.py       9      3    67%
py/desitarget/cmx/ms_cuts.py            341     81    76%
py/desitarget/cuts.py                  1196    154    87%
py/desitarget/gaiamatch.py              547    395    28%
py/desitarget/geomask.py                504    230    54%
py/desitarget/gfa.py                    269    128    52%
py/desitarget/internal/__init__.py        0      0   100%
py/desitarget/internal/sharedmem.py     362    148    59%
py/desitarget/io.py                    1497    972    35%
py/desitarget/lyazcat.py                299    299     0%
py/desitarget/mock/__init__.py            0      0   100%
py/desitarget/mock/io.py                 12     12     0%
py/desitarget/mock/sky.py                38      9    76%
py/desitarget/mtl.py                    960    796    17%
py/desitarget/myRF.py                    97     49    49%
py/desitarget/photo.py                   17     17     0%
py/desitarget/randoms.py                483    438     9%
py/desitarget/secondary.py              419    383     9%
py/desitarget/skybricks.py               60     52    13%
py/desitarget/skyfibers.py              486    246    49%
py/desitarget/skyhealpixs.py             47     47     0%
py/desitarget/subpriority.py             95     70    26%
py/desitarget/sv1/__init__.py             0      0   100%
py/desitarget/sv1/sv1_cuts.py           862    123    86%
py/desitarget/sv1/sv1_targetmask.py      15      6    60%
py/desitarget/sv2/__init__.py             0      0   100%
py/desitarget/sv2/sv2_cuts.py           808    200    75%
py/desitarget/sv2/sv2_targetmask.py      15      6    60%
py/desitarget/sv3/__init__.py             0      0   100%
py/desitarget/sv3/sv3_cuts.py           909    226    75%
py/desitarget/sv3/sv3_targetmask.py      15      6    60%
py/desitarget/targetmask.py              61     13    79%
py/desitarget/targets.py                448     60    87%
py/desitarget/tychomatch.py             148    106    28%
py/desitarget/uratmatch.py              246    187    24%
---------------------------------------------------------
TOTAL                                 13819   6034    56%

============= 100 passed, 5 skipped in 545.60s (0:09:05) =====

The NERSC build/environment is different to the GitHub Actions environment.

geordie666 commented 1 year ago

@weaverba137: Without guidance diagnosing the problem, I won't be able to easily address this issue. This problem doesn't seem to have been introduced in #797, and I see no changes to coverage at NERSC testing in a reasonably constructed environment.

weaverba137 commented 1 year ago

I suspect desitarget needs a general test cleanup pass.

While I'm at it, I noticed many merged or otherwise stale branches that could be deleted. Is there any reason to keep any of the branches that have been merged or were marked closed without merging?

geordie666 commented 1 year ago

I just deleted the merged branches. As to the other stale branches, I can reach out to the authors of those branches and see if they're OK with deleting them.

weaverba137 commented 1 year ago

Could you please specify the exact environment you used to run tests at NERSC? For example, desi_environment.sh 22.5, etc.

geordie666 commented 1 year ago

I use the default. So, whatever is produced by desi_environment.sh without a command line argument. I believe that is equivalent to desi_environment.sh 22.2.

It just occurred to me, though, that, I preempt developer versions of some desihub modules into my PYTHONPATH. So, my environment isn't pristine. I can try running coverage again after only giving precedence to desitarget. If it helps, this is everything, I think:

Currently Loaded Modulefiles:
  1) modules/3.2.11.4                                 15) alps/6.6.67-7.0.3.1_3.29__gb91cd181.ari          29) desisim/0.36.0
  2) craype-network-aries                             16) rca/2.2.20-7.0.3.1_3.27__g8e3fb5b.ari            30) fiberassign/5.4.0
  3) craype-haswell                                   17) atp/3.14.9                                       31) desisurvey/0.18.0
  4) gcc/11.2.0                                       18) perftools-base/21.12.0                           32) surveysim/0.12.3
  5) craype/2.7.10                                    19) PrgEnv-gnu/6.0.10                                33) redrock/0.15.4
  6) cray-mpich/7.7.19                                20) desiconda/20211217-2.0.0                         34) redrock-templates/0.7.2
  7) cray-libsci/20.09.1                              21) desiutil/3.2.5                                   35) prospect/1.2.1
  8) udreg/2.3.2-7.0.3.1_3.24__g5f0d670.ari           22) desitree/0.5.0                                   36) desimeter/0.6.7
  9) ugni/6.0.14.0-7.0.3.1_6.11__g8101a58.ari         23) specter/0.10.0                                   37) simqso/v1.3.0
 10) pmi/5.0.17                                       24) desimodel/0.17.0                                 38) dust/v0_1
 11) dmapp/7.1.1-7.0.3.1_3.28__g93a7e9f.ari           25) specex/0.8.4                                     39) speclite/v0.15
 12) gni-headers/5.0.12.0-7.0.3.1_3.15__gd0d73fe.ari  26) desitarget/2.4.0                                 40) QuasarNP/0.1.3
 13) xpmem/2.2.27-7.0.3.1_3.17__gada73ac.ari          27) specsim/v0.15                                    41) desietc/0.1.14
 14) dvs/2.12_2.2.224-7.0.3.1_3.23__gc77db2af         28) desispec/0.51.13                                 42) desimodules/22.2
weaverba137 commented 1 year ago

@geordie666, note that the installed version of desitarget/2.4.0 is going to be in your PYTHONPATH, unless you have deliberately changed the environment to exclude that. So when a test hits import desitarget, which version is it actually importing?

geordie666 commented 1 year ago

Actually, I'm only using main/developer versions of LSS and legacypipe, which aren't standard DESI dependencies.

geordie666 commented 1 year ago

I'm using the developer version of desitarget. So, '2.5.0.dev5402'.

weaverba137 commented 1 year ago

When you run the test, what is your PYTHONPATH?

geordie666 commented 1 year ago
/global/homes/a/adamyers/git/legacypipe/py:/global/homes/a/adamyers/git/LSS/py:/global/homes/a/adamyers/git/desitarget/py:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desietc/0.1.14/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/QuasarNP/0.1.3/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/speclite/v0.15/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/simqso/v1.3.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desimeter/0.6.7/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/prospect/1.2.1/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/redrock/0.15.4/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/surveysim/0.12.3/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/0.18.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/fiberassign/5.4.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisim/0.36.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desispec/0.51.13/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/specsim/v0.15/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desitarget/2.4.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/specex/0.8.4/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desimodel/0.17.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/specter/0.10.0/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desiutil/3.2.5/lib/python3.9/site-packages:/global/common/software/desi/cori/desiconda//20211217-2.0.0/conda/lib/python3.9/site-packages

(you can see the preemptive versions of desitarget, LSS, legacypipe at the start of the path).

weaverba137 commented 1 year ago

OK, thanx. Just ruling things out.