[WIP] Add GZ3D vac code #733

CKrawczyk · 2021-07-08T10:16:57Z

Adds code for GZ3D VAC integration.

Things left to do:

Ensure all custom class methods work as expected
Document how to use the class
Included docstrings on the class
Properly handle covariance when making stacked spectra
Create examples for using GZ3D masks
Add plotting methods to the GZ3D class for common plots

This pull request:

Has a title that summarises what is changing.
Updates the documentation accordingly.
Has unit tests & code coverage is not adversely affected (within reason).
Works with Python 2.7 and 3.6 (and ideally with Python 3.7).
Updates the CHANGELOG.
Removes more lines of code than it adds.
If relevant, adds a new entry to the What's new? page.

havok2063

Is this still a WIP? I would add docstrings everywhere there isn't it. Complete ones for exposed methods, single line ones for hidden methods. Overall there's a lot going on here. Which are the key methods that you want exposed to the user? I'm guessing the plot methods. Are there methods that return useful data to the user? I would recommend making everything that is really unnecessary to the user hidden.

havok2063 · 2021-09-22T18:52:55Z

python/marvin/contrib/vacs/galaxyzoo3d.py

+    VAC name: <look this up>
+
+    URL: <look this up>
+
+    Description: <look this up>
+


Needs text.

What URL should go here?

python/marvin/contrib/vacs/galaxyzoo3d.py

CKrawczyk · 2021-09-23T08:46:31Z

The WIP is exactly there because of the lack of doc string. I will try to get to this soon, but other projects are eating my time at the moment. Also how important are unit tests for VAC code?

CKrawczyk · 2021-09-23T10:11:03Z

I just added doc strings to the private function, just need to add them to the plotting functions. While working on this code I created an example notebook that shows how to use it. Is this something that can be used in marvin's docs? If so where should it go and what format should it be?

https://gist.github.com/CKrawczyk/191897d179464a8270cda2d055a9f2a0

havok2063 · 2021-09-23T13:55:03Z

Thanks for adding the docstrings! Regarding tests, it's not super important. Our test suite is a bit clunky, out-of-date, and broken at the moment. That being said, if there were some tests you wanted to include as a sanity check, they could go in tests/tools/test_vacs.py. But the tests might not run for you out of the box due to how our suite is setup, but I can either run them myself or tell you how to run them.

The notebook looks great! We can definitely put it in place. It should go in the docs/sphinx/tutorials/notebooks/ folder, and I'd follow the format of the other VAC notebook, VAC_[notebook_name].ipynb. And then add new lines in the docs/sphinx/tutorials/index.rst file. You can test that the docs build by running invoke docs.build from the top level or make html from the docs/sphinx directory. The docs will compile your notebook and convert it to html.

Let me know when you think this is ready to be merged. I'm aiming to have all the VAC PRs merged in the next few weeks so I can begin testing and finalization for DR17.

havok2063 · 2021-11-04T18:31:29Z

@CKrawczyk Are you ready for this to be merged? I need to merge this as soon as possible so I can start finalizing the release of Marvin for DR17 in less than a month.

This PR adds the custom class needed to read in GZ3D data mask fits files.

When reading in a fits table astropy adds blank spaces to the end of short stirngs in a column. For the mangaid search to work these spaces need to be `.strip`ed frist.

This uses the approximation from Westfall 2019 to account for covariance in the spectra when taking averages.

This is a first pass on documenting all the class attributes and methods along with some funcions for plotting the image and masks.

And also with less memory. `_stack_spectra` now uses a running sum for the variance calculation rather than building a full covariance matrix. It also assumes no cov for spaxels with distances more than 6.4 as suggested in Westfall 2019.

Also add abilit to make mask plots in spaxel space.

This is the last of the plotting functions.

Defualt to the correct IFU hexagon and make the `correct_hex=False` give the hexagon shown to the GZ3D volunteers (same as what is baked into the iamge).

All private funcions now have 1-line doc strings.

Finished adding doc strings to all helper functions defined before the main class.

Add the last of the doc strings for the plotting methods of the mian class.

This notebook shows how to use the GZ3D custom class to make various plots.

One of the class methods was a hold over from old code and is no longer needed.

CKrawczyk · 2021-11-15T15:03:58Z

@havok2063 I think this is ready to go, I just need to know what URL you want to be included in the docstring for the GZ3DVAC class?

I have done a test build of the docs locally to make sure the example notebook shows up as expected.

For good measure, I also rebased against the latest vacs_dr17 branch and fixed up any merge conflicts.

havok2063 · 2021-11-15T15:26:51Z

@CKrawczyk Ok, cool. The URL can be whatever URL you want. Most people put a link to the VAC description on the SDSS Wordpress site, but if you have another site to point instead that's fine too.

Ok, great. I'll wait until later in the day to merge this PR in case you want to make any final edits.

In the past we've done only git merges rather than rebases. I don't know if they generally mix well or if that will cause problems with the commit history. If you think it'll be fine then I'm ok with it.

CKrawczyk · 2021-11-15T15:37:40Z

The rebase basically just replayed my commits on top of any there were made to the base branch, any time a conflict was found I resolved it (just in the index.rst of the notebooks doc page).

This will merge into the base branch as if all my commits were made from a fresh branch made today rather than one made several months ago. All in all the git history will look "cleaner" than if I merged the base branch into this branch only for it to be merged back into the base branch.

I will double-check with Karen if we have a VAC URL yet.

havok2063 · 2021-11-15T15:41:10Z

Sounds good to me.

CKrawczyk requested review from albireox, bretthandrews and havok2063 as code owners July 8, 2021 10:16

havok2063 requested changes Sep 22, 2021

View reviewed changes

havok2063 approved these changes Sep 23, 2021

View reviewed changes

CKrawczyk and others added 19 commits November 15, 2021 14:26

Add files for GZ3D VAC

2363c5a

This PR adds the custom class needed to read in GZ3D data mask fits files.

Add comment about covariance calculation

6c36dd2

Fix summary table searching bug

54b29de

When reading in a fits table astropy adds blank spaces to the end of short stirngs in a column. For the mangaid search to work these spaces need to be `.strip`ed frist.

Account for covariance when making mean spectra

78ea464

This uses the approximation from Westfall 2019 to account for covariance in the spectra when taking averages.

Add some docs and plot functions

bfb2d7d

This is a first pass on documenting all the class attributes and methods along with some funcions for plotting the image and masks.

Add bpt plotting functions

46c6996

Calculate mean spectra faster

44f609b

And also with less memory. `_stack_spectra` now uses a running sum for the variance calculation rather than building a full covariance matrix. It also assumes no cov for spaxels with distances more than 6.4 as suggested in Westfall 2019.

More doc strings

740cc80

Adjust some plotting pre-sets

7321d36

Also add abilit to make mask plots in spaxel space.

Add polar plot funcion

3e548ce

This is the last of the plotting functions.

Adjust get_hexagon function

cf60397

Defualt to the correct IFU hexagon and make the `correct_hex=False` give the hexagon shown to the GZ3D volunteers (same as what is baked into the iamge).

Update plot sytles

ccf387d

Add more doc strings

a635618

All private funcions now have 1-line doc strings.

updating mpl11 version in gz3d vac

eb83119

Add doc strings to helper functions

aa275f7

Finished adding doc strings to all helper functions defined before the main class.

Add doc strings to plotting methods

b69e9df

Add the last of the doc strings for the plotting methods of the mian class.

Add example notebook

3d4c250

This notebook shows how to use the GZ3D custom class to make various plots.

Add link to new notebook in index.rst

f688cd9

Remove unused code

54b96cc

One of the class methods was a hold over from old code and is no longer needed.

CKrawczyk force-pushed the feature/add-gz3d-vac branch from 4b7e2fa to 54b96cc Compare November 15, 2021 14:27

Ues context manager for fits file reading

997790f

havok2063 merged commit 442753d into sdss:vacs_dr17 Nov 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add GZ3D vac code #733

[WIP] Add GZ3D vac code #733

CKrawczyk commented Jul 8, 2021 •

edited

Loading

havok2063 left a comment

havok2063 Sep 22, 2021

CKrawczyk Nov 15, 2021

CKrawczyk commented Sep 23, 2021

CKrawczyk commented Sep 23, 2021

havok2063 commented Sep 23, 2021 •

edited

Loading

havok2063 commented Nov 4, 2021

CKrawczyk commented Nov 15, 2021

havok2063 commented Nov 15, 2021

CKrawczyk commented Nov 15, 2021

havok2063 commented Nov 15, 2021

[WIP] Add GZ3D vac code #733

[WIP] Add GZ3D vac code #733

Conversation

CKrawczyk commented Jul 8, 2021 • edited Loading

havok2063 left a comment

Choose a reason for hiding this comment

havok2063 Sep 22, 2021

Choose a reason for hiding this comment

CKrawczyk Nov 15, 2021

Choose a reason for hiding this comment

CKrawczyk commented Sep 23, 2021

CKrawczyk commented Sep 23, 2021

havok2063 commented Sep 23, 2021 • edited Loading

havok2063 commented Nov 4, 2021

CKrawczyk commented Nov 15, 2021

havok2063 commented Nov 15, 2021

CKrawczyk commented Nov 15, 2021

havok2063 commented Nov 15, 2021

CKrawczyk commented Jul 8, 2021 •

edited

Loading

havok2063 commented Sep 23, 2021 •

edited

Loading