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

Optimize API Changes #124

Merged
merged 3 commits into from
Nov 4, 2024
Merged

Optimize API Changes #124

merged 3 commits into from
Nov 4, 2024

Conversation

Tom-Szendrey
Copy link
Contributor

@Tom-Szendrey Tom-Szendrey commented Nov 1, 2024

Issue:

  1. It is hard to follow what is going on as we are throwing all optimize interventions into one big blob.
  2. Our optimize intervention type (find new start time, param value ect) was being incorrectly assigned when multiple types were given all but the first were ignored.

Description

  • Re-organize how we are sending in the interventions to be optimized.
    • Rather than sending one big messy blob send a list of InterventionObjective
  • Utilize intervention_func_combinator to allow us to intervene on two interventions with different types.
    • Previously if we sent the screenshot below both of the interventions were incorrectly using new start time
  • Provide objfun our list of interventions to optimize over as in follow up PR we will be fixing and cleaning the logic written here but I do not want to flood this PR with more changes as it may become very hard to follow.
    The follow up PR will solve the issue that asking to optimize for start time AND value fails

Related Required:

DARPA-ASKEM/terarium#5360

Pictures

image

@Tom-Szendrey Tom-Szendrey marked this pull request as ready for review November 1, 2024 19:29
Copy link
Member

@liunelson liunelson left a comment

Choose a reason for hiding this comment

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

Looks great - thanks for tackling this tricky feature. I see that you've covered the last-day vs. global max QoI, min vs. max objective function and the func_lengths details correctly (at least as far as I know).

@Tom-Szendrey Tom-Szendrey merged commit 93ec358 into main Nov 4, 2024
2 checks 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