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

Adding regularization feature in parmest as an option to the default SSE objective #3550

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sscini
Copy link

@sscini sscini commented Apr 2, 2025

Fixes # .

Summary/Motivation:

Currently, the only default objective is the standard SSE objective. This edit provides the capability to add a
regularization term to the SSE objective with a prior FIM and reference parameter values.

Changes proposed in this PR:

  • Added prior_FIM and theta_ref as keyword arguments for Estimator
  • Added options within Estimator to apply the regularization term to SSE if the new arguments were defined

TODO before converting from draft

  • Receive feedback from collaborators on logical setup
  • Finish edits and debug
  • Confirm functionality with examples

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@sscini
Copy link
Author

sscini commented Apr 2, 2025

@djlaky @adowling2 Please provide early feedback

@sscini sscini changed the title Adding regularization feature as an option to the default SSE objective Adding regularization feature in parmest as an option to the default SSE objective Apr 2, 2025
Copy link
Member

@adowling2 adowling2 left a comment

Choose a reason for hiding this comment

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

@sscini See my early feedback.


Added to SSE objective function
"""
expr = ((theta - theta_ref).transpose() * prior_FIM * (theta - theta_ref) for theta in model.unknown_parameters.items())
Copy link
Member

Choose a reason for hiding this comment

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

Does this run? My intuition is that you need to write out the matrix multiplication and cannot use matrix multiplication.

# Regularize the objective function
second_stage_rule = SSE + regularize_term(prior_FIM = self.prior_FIM, theta_ref = self.theta_ref)
elif self.prior_FIM:
theta_ref = model.unknown_parameters.values()
Copy link
Member

Choose a reason for hiding this comment

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

Why is this line needed?

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