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

DM-42157: Improve model configuration and evaluation #27

Merged
merged 41 commits into from
Mar 14, 2024
Merged

Conversation

taranu
Copy link
Collaborator

@taranu taranu commented Feb 14, 2024

This includes several important bug fixes as well as feature additions.

@taranu taranu force-pushed the tickets/DM-42157 branch 2 times, most recently from 5369a9c to d4e85e9 Compare March 2, 2024 03:45
Copy link

@fred3m fred3m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of small comments, mostly requesting missing underscores to be inserted in variable names.

),
rho=g2f.RhoParameterD(self.rho.value_initial, transform=transform_rho, fixed=self.rho.fixed),
)
integralmodel: g2f.IntegralModel,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very picky detail here, but I think that the parameter should be renamed integral_model.

),
size_y=g2f.ReffYParameterD(
self.size_y.value_initial, transform=transform_size, fixed=self.size_y.fixed
),
rho=g2f.RhoParameterD(self.rho.value_initial, transform=transform_rho, fixed=self.rho.fixed),
)
sersicindex = g2f.SersicMixComponentIndexParameterD(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar, underscore to sersic_index.

python/lsst/multiprofit/errors.py Outdated Show resolved Hide resolved
python/lsst/multiprofit/fit_bootstrap_model.py Outdated Show resolved Hide resolved

@pytest.fixture(scope="module")
def configfitter_psfs(channels) -> dict[g2f.Channel, CatalogExposurePsfBootstrap]:
configdatas = {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config_datas

for results in table_psf_fits.values():
@pytest.fixture(scope="module")
def configdata_sources(
configfitter_psfs, tables_psf_fits,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config_fitter_psfs

def configdata_sources(
configfitter_psfs, tables_psf_fits,
) -> dict[g2f.Channel, CatalogExposureSourcesBootstrap]:
configdatas = {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config_datas

),
n_sources=n_sources,
)
configdata = CatalogExposureSourcesBootstrap(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config_data

return configdatas


def test_fit_psf(configfitter_psfs, tables_psf_fits):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config_fitter_psfs

@taranu taranu force-pushed the tickets/DM-42157 branch from d4e85e9 to 67b5df8 Compare March 14, 2024 02:07
@taranu taranu force-pushed the tickets/DM-42157 branch from 67b5df8 to c1bd744 Compare March 14, 2024 03:52
@taranu taranu merged commit 8b78c75 into main Mar 14, 2024
1 check passed
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