-
Notifications
You must be signed in to change notification settings - Fork 28
Add support for simulation runs #2145
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2145 +/- ##
==========================================
+ Coverage 90.53% 90.59% +0.06%
==========================================
Files 158 159 +1
Lines 23753 23861 +108
==========================================
+ Hits 21504 21618 +114
+ Misses 2249 2243 -6
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, needs the code owner review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main comment is how to deal with creating a SimulationRun by routine and by routine revision.
@overload | ||
def create(self, run: Sequence[SimulationRunWrite]) -> SimulatorRunsList: ... | ||
|
||
def create(self, run: SimulationRunWrite | Sequence[SimulationRunWrite]) -> SimulationRun | SimulatorRunsList: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it's a feature, can it be a separate PR? we are over 500 limit already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expected it to be a separate PR, but I was wondering if these two will have a shared base class which will have consequences for this PR. However, as this is alpha we can do a breaking change later if necessary, so approving for now.
Information for risk-reviewers: The tests failing in this PR have nothing to do with the changes introduced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦄
Description
Every time a simulation routine executes, a simulation run object is created. This object ensures that each execution of a routine is documented and traceable.
Simulation runs provide a historical record of the simulations performed, allowing users to analyze and compare different runs, track changes over time, and make informed decisions based on the simulation results.
Checklist:
If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.