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

Strict bounds for many of the geometry parameters in the HotRegion class use _pi = math.pi. #332

Open
DevarshiChoudhury opened this issue Sep 28, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@DevarshiChoudhury
Copy link
Member

DevarshiChoudhury commented Sep 28, 2023

https://github.com/xpsi-group/xpsi/blob/b705e1772f2cd0d641551c49bb5d8f37ea02d083/xpsi/HotRegion.py#L290C57-L290C57

Instead of math.pi we usually preferably use math.pi - 0.001 for the colatitude to avoid spots being centred on the poles thus leading to no signal modulation, and for angular radii it may be preferable to use math.pi/2.0 - 0.001 since a spot that covers more than half the star is better modelled as elsewhere with a hole. Normally we set these manually. Could it perhaps be better to have them be the default bounds?

The only restriction then arises is if we want to assign spots these values specifically for testing purposes, but I feel these are edge cases. I think it's better to change it for general use and have the users/devs modify it themselves if they really need to.

If we decide to change this, then for colatitudes and super radii, the lower limits can then also be changed from 0.0 to 0.001.

@DevarshiChoudhury DevarshiChoudhury self-assigned this Sep 28, 2023
@DevarshiChoudhury DevarshiChoudhury added question Further information is requested enhancement New feature or request labels Sep 28, 2023
@thjsal
Copy link
Contributor

thjsal commented Sep 29, 2023

The choice of 0.001 seems a bit arbitrary to me though. Why not allow the user to set it to 0.00000001? It is also not clear to me why the limit could not be exactly 0.0 (or math.pi/2.0), since during sampling it is very unlikely that you would get a sample precisely from the prior boundary, right?

Or if we are sure that this 0.001 is really the limit after which numerical problems arise, maybe we could indeed set the hard bounds to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants