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

Use of SED grid 'keep' column? #73

Closed
karllark opened this issue Feb 25, 2017 · 11 comments
Closed

Use of SED grid 'keep' column? #73

karllark opened this issue Feb 25, 2017 · 11 comments
Assignees
Labels

Comments

@karllark
Copy link
Member

The SED grid has 'keep' column. Is this column used anywhere? It has changed as revealed by the regress_check code. Cannot find where it is used. If not used, should we delete? If not used, should it be?

@mfouesneau
Copy link
Member

I believe it was initially used to keep track of extrapolation then I guess we used it to see which library was used when combining multiple ones.

@karllark
Copy link
Member Author

Still seems to be about extrapolation. Code snippet from physicsmodel/creategrid.py

    # Step 2: Avoid Extrapolation
    # ===========================
    # check boundary conditions, keep the data but do not compute the
    #    sed if not needed
    bound_cond = osl.points_inside(zip(r['logg'], r['logT']))
    _grid['keep'][start_idx: end_idx] = bound_cond[:]

And then it is used in the same file:

# Step 5: filter points without spectrum
# ======================================
#filter unbound values
idx = np.array(_grid.pop('keep'))

specs = specs.compress(idx, axis=0)
for k in _grid.keys():
        _grid[k] = _grid[k].compress(idx, axis=0)

But the keep column is still in the grid file on disk and it has both True and False values.

So, not sure it is being used correctly or what.

@mfouesneau
Copy link
Member

Maybe just remove the column once produced? It's surprising to me that the final stored data will have False values left...

@karllark
Copy link
Member Author

I feel we should do a little more debugging to make sure it is being used correctly before removing it after the compress step. Need to look at that step to see what it is doing with the compress.

@mfouesneau
Copy link
Member

compress is just a selection based on keep. What's surprinsing is that the key keep is popped out first and never put back. I don't understand how it reappears later, do you?

@karllark
Copy link
Member Author

Nope. Hence the need for some debugging.

@galaxyumi
Copy link
Contributor

I will take a look at it tomorrow as well. The fact is though none of us have used that column.

@karllark
Copy link
Member Author

Working on regression testing, I investigated this a bit more. It is used internally in gen_spectral_grid_from_stellib in the creategrid.py file. This is a deprecated function. Regardless, I've added a line in this function to delete the column once it has been used internally.

I have removed creating this column in the init function in stelllib. When created, it is empty. This explains why it would show changes in the regression tests. It was not initialized so was filled with random True/False values.

These two changes should mean that the keep column no longer appears in the spectral grid file. This removes the h5py incompatible datatype.

@galaxyumi
Copy link
Contributor

Great!

@karllark
Copy link
Member Author

Keep column will be removed in pull request #140.

@karllark
Copy link
Member Author

Done!

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

No branches or pull requests

3 participants