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

Fix for issue #81: Add warning if w > 1. #97

Merged
merged 4 commits into from Mar 26, 2014
Merged

Conversation

thegyro
Copy link
Contributor

@thegyro thegyro commented Mar 14, 2014

throw a warning if dilution factor is greater than 1

@wkerzendorf
Copy link
Member

@thegyro So you see that it is an array like structure, which is great! but it's a numpy array. have a look for functions that test if any of the values are greater than 1 and then just issue one warning.

@thegyro
Copy link
Contributor Author

thegyro commented Mar 14, 2014

Thanks for the reply! I am iterating over the ws array which was used in the statement self.ws = numpy.array(ws). So the ws I am iterating over is just a normal array right ? (That is I am iterating over a normal array ,which was used to create a numpy array )

@wkerzendorf
Copy link
Member

@thegyro: It's an array, so what you have does work! But python for-loops are incredibly slow and thus it is good to try to use the numpy framework (read up on numpy). It would also be a little bit annoying for a 72 shell model getting a warning 50 times that one of the values is >1. I think the best way forward here is to make a logger warning that details the number of offending values (i.e. "5/72 values were greater than 1").

@thegyro
Copy link
Contributor Author

thegyro commented Mar 14, 2014

Ah! Thanks for the clarification! I have tried a better way (hopefully) to check for warning > 1. Please let me know if this is correct.

@wkerzendorf
Copy link
Member

Close - use np.any. But good fix. if you can think about, how to test this, even better. Otherwise I'll let @ssim have a quick look and if he's fine we'll merge.

@thegyro
Copy link
Contributor Author

thegyro commented Mar 14, 2014

@wkerzendorf Is it alright now ?

@wkerzendorf
Copy link
Member

that looks good @ssim, have a quick look and merge if you agree.

@ssim
Copy link
Contributor

ssim commented Mar 17, 2014

@mklauser do you have a moment to look at this? I'm a bit swamped right now.

@ssim
Copy link
Contributor

ssim commented Mar 26, 2014

We're happy - thank you! - merge now.

ssim added a commit that referenced this pull request Mar 26, 2014
Fix for issue #81: Add warning if w > 1.
@ssim ssim merged commit a2be47d into tardis-sn:master Mar 26, 2014
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.

None yet

3 participants