Skip to content

Comments

fixing LD bug for joint fits#744

Merged
taylorbell57 merged 5 commits intokevin218:mainfrom
erinmmay:main
Jun 12, 2025
Merged

fixing LD bug for joint fits#744
taylorbell57 merged 5 commits intokevin218:mainfrom
erinmmay:main

Conversation

@erinmmay
Copy link
Collaborator

addressing bug #743

I believe this is how we want to do it, limb darkening should have channel id appended regardless of if it's a free fit since we can fix them to different values.

@erinmmay
Copy link
Collaborator Author

also adding a minor fix for fleck. Currently if you set the star rotation (spotrot) in the s5 fit params epf file then fleck_fast is never set to anything and the code crashes, this adds the lines to set fleck_fast when spotrot is defined in the epf

@taylorbell57
Copy link
Collaborator

I haven't yet confirmed where this bug arises, but as it stands I don't think this patch is quite right. If there's a bug (which I'm certainly willing to believe, though it hasn't arisen for non-multwhite fits), it must be in the interactions between these snippets of code:

# Load in all the values for each astro parameter
for item in self.__dict__.keys():
item0 = item+self.pid_id
try:
if model.parameters.dict[item0][1] == 'free':
item0 += self.channel_id
value = getattr(parameterObject, item0)
if eval:
value = value.value
setattr(self, item, value)
except KeyError:
if (item in [f'u{i}' for i in range(1, 5)] or
'spot' == item[:4]):
# Limb darkening and spots probably don't vary with planet
try:
item0 = item
if model.parameters.dict[item0][1] == 'free':
item0 += self.channel_id
value = getattr(parameterObject, item0)
if eval:
value = value.value
setattr(self, item, value)
except KeyError:
pass
else:
pass

ld_func = ld_profile(self.limb_dark)
len_params = len(inspect.signature(ld_func).parameters)
coeffs = ['u{}'.format(n) for n in range(1, len_params)]
self.u = [getattr(self, coeff) for coeff in coeffs]

# Store the ld_profile
self.ld_from_S4 = kwargs.get('ld_from_S4')
ld_func = ld_profile(self.parameters.limb_dark.value,
use_gen_ld=self.ld_from_S4)
len_params = len(inspect.signature(ld_func).parameters)
self.coeffs = ['u{}'.format(n) for n in range(1, len_params)]
self.ld_from_file = kwargs.get('ld_from_file')
# Replace u parameters with generated limb-darkening values
if self.ld_from_S4 or self.ld_from_file:
log.writelog("Using the following limb-darkening values:")
self.ld_array = kwargs.get('ld_coeffs')
for c in range(self.nchannel_fitted):
chan = self.fitted_channels[c]
if self.ld_from_S4:
ld_array = self.ld_array[len_params-2]
else:
ld_array = self.ld_array
for u in self.coeffs:
index = np.where(np.array(self.paramtitles) == u)[0]
if len(index) != 0:
item = self.longparamlist[c][index[0]]
param = int(item.split('_')[0][-1])
ld_val = ld_array[chan][param-1]
log.writelog(f"{item}: {ld_val}")
# Use the file value as the starting guess
self.parameters.dict[item][0] = ld_val
# In a normal prior, center at the file value
if (self.parameters.dict[item][-1] == 'N' and
self.recenter_ld_prior):
self.parameters.dict[item][-3] = ld_val
# Update the non-dictionary form as well
setattr(self.parameters, item,
self.parameters.dict[item])

If the bug is specifically with loading-in exotic-ld results (and/or custom ld files), then my pretty strong suspicion is that the bug is in the last of those three snippets (from BatmanModels.py or the equivalent chunks of code for fleck or starry). I won't have time to look into this more today, but I can try to investigate further tomorrow if you don't beat me to it.

@taylorbell57 taylorbell57 self-requested a review February 11, 2025 17:09
@taylorbell57 taylorbell57 added bug Something isn't working LC Fit labels Feb 11, 2025
@erinmmay
Copy link
Collaborator Author

erinmmay commented Feb 11, 2025

I added tons of print statements to confirm that the code you reference in BatmanModels sets things up in properly when reading from the exotic values and that the bug arises when BatmanModels.py calls AstroModel/PlanetParams to initialize the planet. AstroModel line 308/309 was previously only looking for u1/u2, never u1_ch1/u2_ch1 and the result was that both visits used the same limb darkening (confirmed with the print statements, visually with the plots, and by switching the order of the visits to see which LD values were used in that case)

@taylorbell57
Copy link
Collaborator

I really would've thought that the ld parameters would be populated with the values for ch1 based on the snippet below which is used to set (in the pretend case of some parameter called par) the value of PlanetParams.par to the value of parameterObject.par_ch#:

try:
if model.parameters.dict[item0][1] == 'free':
item0 += self.channel_id
value = getattr(parameterObject, item0)
if eval:
value = value.value
setattr(self, item, value)

It is the same snippet that is used for every other variable, with the only exception being that lower down there's a snippet that allows ld parameters and spot parameters to be the same between different planets.

@taylorbell57
Copy link
Collaborator

The lines to which you are referring:

coeffs = ['u{}'.format(n) for n in range(1, len_params)]
self.u = [getattr(self, coeff) for coeff in coeffs]

use self.u1 which should've been set to the value of parameterObject.u1_ch1 by those lines I just linked above.

@erinmmay
Copy link
Collaborator Author

yeah unfortunately they are not set to parameterObject.u1_ch1. self.u1 returns the actual u1 value. If there's a different way you want to fix this happy to do that instead, but as it stands it's not working as expected.

@erinmmay
Copy link
Collaborator Author

erinmmay commented Feb 11, 2025

Ahh I see the issue. Lines 149-155 in AstroModel that you linked only set self.u1 to parameterObject.u1_ch1 if the parameter is free. But We often fix to the exotic-values, in which case it doesn't append the channel id.

@taylorbell57
Copy link
Collaborator

Ahh yeah, that'd do it! Honestly maybe we don't need that 'free' check at all since its already wrapped in a KeyError check, though maybe we still do (worth investigating).

@taylorbell57
Copy link
Collaborator

@erinmmay, can you click the "allow maintainers to push edits to this pull request" (or whatever the exact wording is) button so that I can make some small formatting tweaks at some point tomorrow?

@erinmmay
Copy link
Collaborator Author

On my end it says that's already enabled.

@taylorbell57
Copy link
Collaborator

Okay thanks, must just be an issue with the online GitHub Codespaces then - I'll try using my local git installation later

@taylorbell57
Copy link
Collaborator

@erinmmay, I just reworked this patch to (hopefully) more generally catch similar issues with fixed parameters during shared or multwhite fits. Can you please test this new code and make sure it still resolves your original problem?

@taylorbell57
Copy link
Collaborator

@erinmmay, have you had the chance to test my edits to this PR? I'd like to get this patch merged if it's good-to-go

@taylorbell57 taylorbell57 requested a review from kevin218 June 12, 2025 08:15
@taylorbell57
Copy link
Collaborator

@kevin218, do you want to review this PR - it works for me, but would be good to get a second set of eyes on this

@github-project-automation github-project-automation bot moved this from In progress to Reviewer approved in Road to v2.0 Jun 12, 2025
@taylorbell57 taylorbell57 merged commit 64f85c7 into kevin218:main Jun 12, 2025
3 of 4 checks passed
@github-project-automation github-project-automation bot moved this from Reviewer approved to Done in Road to v2.0 Jun 12, 2025
@github-project-automation github-project-automation bot moved this from In progress to Done in Stage 5: Light Curve Fitting Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working LC Fit

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Limb darkening for joint fits not using different values for the different visits/channels

3 participants