-
Notifications
You must be signed in to change notification settings - Fork 25
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
✨ Added timeout for QuickSim
#654
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: GitHub Actions <[email protected]>
Signed-off-by: GitHub Actions <[email protected]>
Signed-off-by: GitHub Actions <[email protected]>
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.
clang-tidy made some suggestions
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Outdated
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
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.
clang-tidy made some suggestions
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
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.
clang-tidy made some suggestions
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #654 +/- ##
==========================================
- Coverage 98.41% 98.40% -0.01%
==========================================
Files 252 252
Lines 40856 40880 +24
Branches 1863 1865 +2
==========================================
+ Hits 40207 40228 +21
- Misses 649 652 +3
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Signed-off-by: GitHub Actions <[email protected]>
// measure run time (artificial scope) | ||
// main loop |
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.
This comment (// main loop
) seems to be misplaced
/** | ||
* Timeout limit (in ms). | ||
*/ | ||
uint64_t timeout = std::numeric_limits<uint64_t>::max(); |
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 believe, we usually specify our timeouts in seconds. Please check exact
and gold
to be sure.
if (ps.timeout == 0) | ||
{ | ||
st.additional_simulation_parameters.emplace("timeout_reached", true); | ||
return st; | ||
} |
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.
Is it really a simulation parameter that the timeout has been reached? In exact
, we throw an exception in the _impl
class that we then catch in the calling function exact
and return an std::nullopt
. I'm not 100% sure how gold
handles this. You might wanna check.
mockturtle::stopwatch<>::duration time_counter{}; | ||
|
||
// Track the start time for timeout | ||
auto start_time = std::chrono::high_resolution_clock::now(); |
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.
Could be const
if (timeout_limit_reached) | ||
{ | ||
st.additional_simulation_parameters.emplace("timeout_reached", true); | ||
} |
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.
See above
Description
This PR adds the option to set a timeout in QuickSim to avoid endless simulations.
Checklist: