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

Make centroid offset for low separations optional #9

Merged
merged 3 commits into from
Mar 5, 2019

Conversation

jakeown
Copy link

@jakeown jakeown commented Mar 4, 2019

No description provided.

@jakeown
Copy link
Author

jakeown commented Mar 4, 2019

Centroid offset optional

Jared Keown added 2 commits March 4, 2019 14:25
Creates balanced training set with 1comp, noise, and 2comp classes
If True, will output the central pixel spectrum and the 3x3 average spectrum as an H5 file. A second H5 file with the corresponding class labels is also created.
mcyc
mcyc previously approved these changes Mar 4, 2019
@mcyc mcyc self-requested a review March 4, 2019 23:28
@mcyc mcyc dismissed their stale review March 4, 2019 23:35

I made a mistake of approving all 3 commits when I only intended to approve the first commit

@low-sky
Copy link
Contributor

low-sky commented Mar 5, 2019

Good to go?

@mcyc
Copy link
Collaborator

mcyc commented Mar 5, 2019

Functionally, I think so. Perhaps make outputting (2,2) lines optional?

I have plans to modularize the code and enable multi-processing, as I did with my own version of the code, but that is not necessary for our test.

@low-sky
Copy link
Contributor

low-sky commented Mar 5, 2019

Yes; I'm seeing that we're overloading this script with a lot of use cases, which are obviating the main purpose of providing a common set of tests. Factoring out the cube generation and having a set of routines that call pieces of this code seems like it would be preferred over putting everything in one script.

@mcyc
Copy link
Collaborator

mcyc commented Mar 5, 2019

Agreed, that's what I have in mind as well.

@mcyc
Copy link
Collaborator

mcyc commented Mar 5, 2019

Let's merge the pull request first. I can start modularizing the code following that.

@mcyc mcyc merged commit 2b79aeb into GBTAmmoniaSurvey:master Mar 5, 2019
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