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

lsm predict requires SPI fit #31

Closed
bennahugo opened this issue Sep 26, 2020 · 6 comments
Closed

lsm predict requires SPI fit #31

bennahugo opened this issue Sep 26, 2020 · 6 comments
Labels
bug Something isn't working

Comments

@bennahugo
Copy link
Collaborator

If the SPI is not present (ie. flat spectrum assumed) the following error is generated:

ValueError: Reference frequency not found for source A171G in /home/hugo/workspace/DEEP2_tagged.lsm.html. Please set reference frequency for either this source or the entire file.

I think it is much better to assume a constant spectrum than not to allow the user to predict from non-SPI-fitted models

@bennahugo bennahugo added the bug Something isn't working label Sep 26, 2020
@bennahugo
Copy link
Collaborator Author

Case in point is the narrow band subset I'm testing with

@JSKenyon
Copy link
Collaborator

I am not totally convinced. I think I put this check in because I don't want things to be assumed. That is how mistakes get made. If users want everything to be flat spectrum, I don't think it is unfair to expect them to put that in the model. I am open to counter-arguments though.

@bennahugo
Copy link
Collaborator Author

It is not standard though @JSKenyon. Unless you explicitly fit for spis with bdsf the the reference frequency is left to None and the spi column just does not exist for any of the components. Therefore a flat spectrum is implicitly assumed by the model. I agree that if at least one component has an spi either the global frequency or the component frequency must be set.

@bennahugo
Copy link
Collaborator Author

bennahugo commented Sep 26, 2020

You already assume spi = 0 if the attribute does not exist so (nu / refnu) ^ (0.0) == 1 so the reference frequency can be set to anything non-zero - I've updated the commit to reflect this

@JSKenyon
Copy link
Collaborator

JSKenyon commented Sep 1, 2021

This will likely be addressed/improved when #113 is in place.

@JSKenyon
Copy link
Collaborator

Flat spectrum sources (and models) should now be handled correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants