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

[TASK] Sampler/Optimizer function execution order #2302

Closed
10 tasks done
aalfonsi opened this issue Apr 17, 2024 · 4 comments · Fixed by #2319
Closed
10 tasks done

[TASK] Sampler/Optimizer function execution order #2302

aalfonsi opened this issue Apr 17, 2024 · 4 comments · Fixed by #2319
Assignees
Labels
priority_minor task This tag should be used for any new capability, improvement or enanchment

Comments

@aalfonsi
Copy link
Collaborator

aalfonsi commented Apr 17, 2024


Issue Description

Is your feature request related to a problem? Please describe.
For complex problems, a set of functions (in Samplers/Optimizers) might depend on another one/another set. Currently there is no way in RAVEN to specify an order of execution or for RAVEN to infer the order of execution as function of the input/output variables.

Describe the solution you'd like
2 options are possible:

  • Easiest to implement: allow the user to specify an order of execution of the variables/functions evaluation
  • More rubust: usage of graph theory (EnsembleModel-like) to infer the order of execution.

Describe alternatives you've considered
In all the functions that depend on another function/function set, the code snippet needed to compute the "common quantity" is copied pasted in each function.


For Change Control Board: Issue Review

This review should occur before any development is performed as a response to this issue.

  • 1. Is it tagged with a type: defect or task?
  • 2. Is it tagged with a priority: critical, normal or minor?
  • 3. If it will impact requirements or requirements tests, is it tagged with requirements?
  • 4. If it is a defect, can it cause wrong results for users? If so an email needs to be sent to the users.
  • 5. Is a rationale provided? (Such as explaining why the improvement is needed or why current code is wrong.)

For Change Control Board: Issue Closure

This review should occur when the issue is imminently going to be closed.

  • 1. If the issue is a defect, is the defect fixed?
  • 2. If the issue is a defect, is the defect tested for in the regression test system? (If not explain why not.)
  • 3. If the issue can impact users, has an email to the users group been written (the email should specify if the defect impacts stable or master)?
  • 4. If the issue is a defect, does it impact the latest release branch? If yes, is there any issue tagged with release (create if needed)?
  • 5. If the issue is being closed without a pull request, has an explanation of why it is being closed been provided?
@aalfonsi aalfonsi added priority_minor task This tag should be used for any new capability, improvement or enanchment labels Apr 17, 2024
@aalfonsi
Copy link
Collaborator Author

aalfonsi commented Apr 17, 2024

@wangcj05 @mandd @PaulTalbot-INL @joshua-cogliati-inl ALL what do you think?

@wangcj05
Copy link
Collaborator

@aalfonsi Both are good for me. If you choose the option two, I think we need to switch "variables" of "Functions" to "Inputs"/"Outputs" instead.

@alfoa
Copy link
Collaborator

alfoa commented Apr 17, 2024

@aalfonsi Both are good for me. If you choose the option two, I think we need to switch "variables" of "Functions" to "Inputs"/"Outputs" instead.

Oh Yes absolutely that can be done to be more explicit. Implicitly in case of "functions" i think the input output relation is in their definition.
For example in a case like the one below:

<Functions>
  <External file="myFile.py" name="myFunction">
    <variables>sampleVar1,variablefromOtherFunction</variables>
  </External>
</Functions>

<variable name="myVar">
  <function>myFunction</function>
</variable>

The output is myVar (the variable name) and the inputs are the Functions variables.

@wangcj05
Copy link
Collaborator

wangcj05 commented Jun 7, 2024

issue closure checklist is good.

wangcj05 added a commit that referenced this issue Jun 7, 2024
…ers (#2319)

* moved samplers-optimizers function tests in their own folder for clarity

* renamed

* modified test names and revisions

* moved subfolders

* Closes #2302

* usage base class function evaluation method in ensembleForward

* added error msg for detection of loop in function system

* added test for ensemble model passing strings (restart file paths) around

* removed trailing spaces

* Update ravenframework/Samplers/EnsembleForward.py

* Update ravenframework/Samplers/Sampler.py

* Update ravenframework/Samplers/Sampler.py

* Update ravenframework/Samplers/Sampler.py

* fixed comment

* we always check for isolated functions

* updated model.tex

* changed order to reflect order of appearance in the introduction of the Model sections

* modified test description to make them latex compatible

* specialization for EnsembleForward and CustomSampler

* Apply suggestions from code review

addressed Congjian's comments

* Apply suggestions from code review

* Apply suggestions from code review

* Update ravenframework/Samplers/Sampler.py

* updated setuptools dep

* updated to simply ver 69

* added utility function as Congjian's request

* plot entity

* model order

* added starting models

* Apply suggestions from code review

* Update ravenframework/utils/graphStructure.py

---------

Co-authored-by: Congjian Wang - INL <[email protected]>
Jimmy-INL pushed a commit that referenced this issue Jul 7, 2024
…ers (#2319)

* moved samplers-optimizers function tests in their own folder for clarity

* renamed

* modified test names and revisions

* moved subfolders

* Closes #2302

* usage base class function evaluation method in ensembleForward

* added error msg for detection of loop in function system

* added test for ensemble model passing strings (restart file paths) around

* removed trailing spaces

* Update ravenframework/Samplers/EnsembleForward.py

* Update ravenframework/Samplers/Sampler.py

* Update ravenframework/Samplers/Sampler.py

* Update ravenframework/Samplers/Sampler.py

* fixed comment

* we always check for isolated functions

* updated model.tex

* changed order to reflect order of appearance in the introduction of the Model sections

* modified test description to make them latex compatible

* specialization for EnsembleForward and CustomSampler

* Apply suggestions from code review

addressed Congjian's comments

* Apply suggestions from code review

* Apply suggestions from code review

* Update ravenframework/Samplers/Sampler.py

* updated setuptools dep

* updated to simply ver 69

* added utility function as Congjian's request

* plot entity

* model order

* added starting models

* Apply suggestions from code review

* Update ravenframework/utils/graphStructure.py

---------

Co-authored-by: Congjian Wang - INL <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority_minor task This tag should be used for any new capability, improvement or enanchment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants