desihub / desitarget

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

restore select_mock_targets to working order #788

Closed moustakas closed 2 years ago

moustakas commented 2 years ago

[WIP]

Building on #786, and on the heals of the major fixes in the template-generating code in https://github.com/desihub/desisim/pull/559, the purpose of this PR is to restore select_mock_targets to working order (it's been broken for many months, if not a year) and to address a number of open bugs and issues, including #735, #745, #747, and others.

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 56.466% when pulling 83c8a2421dcb6f91550b717804e57c7985cfcf0d on mock-fixes into d26a11443d93328f6579b95583d35e2621416768 on master.

moustakas commented 2 years ago

@geordie666 @weaverba137 @sbailey this PR is now ready for review. However, I need help setting up the unit tests to test the mock code (specifically, test/test_mock_build), which will need to bring in desisim@master as a dependency (at least until we cut a new tag).

For the record, the mock unit tests pass with a local check-out of this branch using

source /global/cfs/cdirs/desi/software/desi_environment.sh master

A couple quick notes:

weaverba137 commented 2 years ago

For the record: it looks like desitarget already depends on desisim, and of course, desisim already depends on desitarget. Thus we have a circular dependency. It hasn't caused problems so far, but those are always a potential source of pain.

For the sharedmem issue, it would be interesting to compare what version of Python you are using on your laptop to the version at NERSC. If you're using the DESI software stack at all, I think that is still Python 3.8, which would appear to contradict the ticket.

moustakas commented 2 years ago

For the sharedmem issue, it would be interesting to compare what version of Python you are using on your laptop to the version at NERSC. If you're using the DESI software stack at all, I think that is still Python 3.8, which would appear to contradict the ticket.

On my laptop I was using Python 3.8.12 vs Python 3.8.3 for the DESI software stack. I didn't dig any deeper into the issue other than to celebrate when everything "just worked" after I switched from my laptop to NERSC.