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: deBroeg std issue #113

Merged
merged 2 commits into from
Jul 18, 2023
Merged

fix: deBroeg std issue #113

merged 2 commits into from
Jul 18, 2023

Conversation

schackey
Copy link
Contributor

Before, the standard deviation was applied on the mean difference between current and last weights (scalar), which resulted in the evolution parameter being zero already in the first iteration. Hence, the deBroeg algorithm only runs once when using .autodiff(). Removing the standard deviation fixes the issue.

I also tested between using the maximum difference and the mean difference of current and last weight for a few example dates. In the dates I've tested, the results are identical, but the mean difference method uses less iterations. This is why I have settled for the mean difference approach for now.

Furthermore, I've implemented two additional lines related to any mask that is applied to a 'Fluxes' instance by using the 'mask_stars' function. Specifically, i identify stars with a std of zero in the first iteration (as 'mask_stars' sets masked stars to be -1 for all times) and then apply the mask after all iterations to the resulting weights. I did this, as before it would only set the weights to zero for these masked stars in the first iteration, but as soon as the weights are defined by the binned white noise of the lightcurve (from the second iteration onwards), the masked stars get non-zero values, which defies the purpose of masking.

schackey and others added 2 commits July 15, 2023 23:53
The use of the standard deviation on the evolution parameter resulted in it being zero after the first iteration, leading to the while-loop breaking in the first iteration every time.
@lgrcia
Copy link
Owner

lgrcia commented Jul 18, 2023

That is a great fix @schackey (cant' believe this was left in the code...). I just made a small chore commit on your fork to patch the version number.

@lgrcia lgrcia merged commit be782e1 into lgrcia:main Jul 18, 2023
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.

2 participants