Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor QSO_templates #67

Merged
merged 11 commits into from
Feb 3, 2016
Merged

Refactor QSO_templates #67

merged 11 commits into from
Feb 3, 2016

Conversation

profxj
Copy link
Contributor

@profxj profxj commented Jan 28, 2016

Refactored QSO templates a bit

Added ability to generate a new set of tempaltes 'on-the-fly'
Pushed previous documentation into desisim, and updated
No unit tests (yet)

Failing travis, but can't be due to this code (I think)
There may be hooks to xastropy, but not in the code
intended for others.
Requires a new data file, to be provided on NERSC

@profxj
Copy link
Contributor Author

profxj commented Jan 28, 2016

Also, this has a dependency on PR#18
in desiutils.

@sbailey
Copy link
Contributor

sbailey commented Jan 28, 2016

tests are failing due to desiutil 1.3 setuptools.compat bug fixed with desiutil 1.3.2 . desisim/master already has that from another PR a few minutes ago; I will merge master into qso_templates, push that back here, and we'll go from there.

Yeowch:

======================================================================
ERROR: desisim.qso_template.tests (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: desisim.qso_template.tests
Traceback (most recent call last):
  File "/Users/sbailey/anaconda/lib/python2.7/unittest/loader.py", line 254, in _find_tests
    module = self._get_module_from_name(name)
  File "/Users/sbailey/anaconda/lib/python2.7/unittest/loader.py", line 232, in _get_module_from_name
    __import__(name)
  File "/Users/sbailey/desi/git/desisim/py/desisim/qso_template/tests.py", line 88, in <module>
    eigen_fil = os.environ.get('IDLSPEC2D_DIR')+'/templates/spEigenQSO-55732.fits'
TypeError: unsupported operand type(s) for +: 'NoneType' and 'unicode'

We don't want to have an IDLSPEC2D dependency, even if that is just for a test. Could you copy that into $DESI_BASIS_TEMPLATES (e.g. at NERSC /project/projectdirs/desi/spectro/templates/basis_templates/v1.1) and then wrap the test to skip if $DESI_BASIS_TEMPLATES isn't defined?

@profxj
Copy link
Contributor Author

profxj commented Jan 29, 2016

I've just commented that code out. It was really for
development only.

@sbailey
Copy link
Contributor

sbailey commented Jan 29, 2016

Travis is complaining that the coverage went down significantly, but this is an artifact of us skipping most tests due to not having lightweight templates available in the travis environment. The actual coverage is pretty good:

Name                   Stmts   Miss  Cover   Missing
----------------------------------------------------
desisim                    7      0   100%   
desisim.filterfunc        31      1    97%   33
desisim.io               259      6    98%   467, 515-516, 531-534
desisim.obs              154      2    99%   276, 296
desisim.pixelsplines     176    121    31%   28-50, 80, 83, 87, 131, 133, 153-178, 185-201, 204-217, 219-226, 232-293, 308-319, 323-343
desisim.pixsim           115     13    89%   44, 60, 74, 82, 86, 105, 138-140, 153, 178-179, 181
desisim.qso_template       0      0   100%   
desisim.targets          269     86    68%   307-312, 314, 316-327, 338-342, 351-356, 363, 373-382, 386-393, 402-410, 412, 426-437, 455-488
desisim.templates        492     23    95%   29, 56-62, 392-393, 395-396, 528, 531-533, 600-601, 706, 724, 730, 957, 1144, 1150
----------------------------------------------------
TOTAL                   1503    252    83%   
----------------------------------------------------------------------
Ran 24 tests in 52.917s

it seems to have no coverage of py/desisim/qso_template itself, but if I understand correctly that is mainly for generating the DESI coefficients using BOSS data rather than generating the DESI templates on the fly.

@profxj
Copy link
Contributor Author

profxj commented Feb 3, 2016

Connected template.QSO to desisim.qso_templates.desi_qso_templ.desi_qso_templates()
Added a Notebook for 'testing'

@sbailey
Copy link
Contributor

sbailey commented Feb 3, 2016

Tests weren't passing since code was looking for QSOs basis templates in desispec/data/ instead of $DESI_BASIS_TEMPLATES like all the other classes. I updated the code to treat the QSOs like the other basis templates but the random number seed test is still failing, i.e.

from desisim.templates import QSO
qso = QSO()
flux1, wave1, meta1 = qso.make_templates(5, seed=1)
flux2, wave2, meta2 = qso.make_templates(5, seed=1)
flux1==flux2

--> an array of Falses intead of Trues.

Punting this back to @profxj to fix the random number seed stuff.

@profxj
Copy link
Contributor Author

profxj commented Feb 3, 2016

Running on my laptop:

python test_templates.py

Gives


Ran 5 tests in 30.311s

OK (expected failures=1)

@sbailey
Copy link
Contributor

sbailey commented Feb 3, 2016

Tests work again with both v1.1 and v2.0 basis templates. Merging. Thanks for the refactor to generate QSO templates on the fly.

sbailey added a commit that referenced this pull request Feb 3, 2016
@sbailey sbailey merged commit 918893f into master Feb 3, 2016
@moustakas
Copy link
Member

@profxj Could you take a quick look and see if desisim/bin/simulate-templates also needs to be updated, given your changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants