[MISC] Speed up solver via linesearch/update_constraint/func_init optimizations#2445
[MISC] Speed up solver via linesearch/update_constraint/func_init optimizations#2445erizmr wants to merge 14 commits intoGenesis-Embodied-AI:mainfrom
Conversation
erizmr
force-pushed
the
mingrui/260218/solver_opt_merge_to_main
branch
from
b7969ff to
912fdf6
Compare
erizmr
force-pushed
the
mingrui/260218/solver_opt_merge_to_main
branch
from
912fdf6 to
0d43c2b
Compare
erizmr
force-pushed
the
mingrui/260218/solver_opt_merge_to_main
branch
from
034c2e5 to
e8422e9
Compare
|
🔴 Benchmark Regression Detected ➡️ Report |
|
🔴 Benchmark Regression Detected ➡️ Report |
|
🔴 Benchmark Regression Detected ➡️ Report |
| @qd.func | ||
| def _log_scale(min_value: gs.qd_float, max_value: gs.qd_float, num_values: qd.i32, i: qd.i32) -> gs.qd_float: | ||
| step = (qd.log(max_value) - qd.log(min_value)) / qd.max(1.0, gs.qd_float(num_values - 1)) | ||
| return qd.exp(qd.log(min_value) + gs.qd_float(i) * step) |
There was a problem hiding this comment.
This should be moved in generic quadrants kernel utils
There was a problem hiding this comment.
This is only for linesearch use. I intend to add a note to state it can be moved in generic quadrants kernel utils and do follow up PRs on it. How do you think?
There was a problem hiding this comment.
If think it should either be inlined or move in generic quadrants kernel utils. No good reason to wait.
| n_entities: int = -1 | ||
| n_links: int = -1 | ||
| n_geoms: int = -1 | ||
| iterations: int = -1 |
There was a problem hiding this comment.
Remove this. It is not the right place. We do consider dynamically changing variables as pre-compiled constant. Moreover, "iterations" is a terrible variable name.
There was a problem hiding this comment.
This is for removing the reading from field rigid_global_info.iterations[None] in python scope, i.e., removing the implicit cpu-gpu sync. I have two other solutions in mind: 1. Store as a Python attribute on rigid_global_info 2. add a new parameter such n_solver_iterations to func_solve_body, which one would you prefer? Or feel free to share a three
There was a problem hiding this comment.
I need to think about it. At this point none of them seems acceptable.
There was a problem hiding this comment.
Could you share more details regarding what this attribute is storing and how it is used? Is it changing at every simulation step or is it a constant? How does it works?
There was a problem hiding this comment.
If it is a constant, the usual approach is to store another Python native attribute with _ prefix in the solver and pass it as input argument of Python-native functions only. We are doing this for instance to skip kernel calls for specialized collision detection if useless, eg box-box.
| from genesis.engine.solvers.rigid.constraint import solver | ||
|
|
||
| LS_PARALLEL_K = 16 | ||
| LS_PARALLEL_MIN_STEP = 1e-6 |
There was a problem hiding this comment.
This should depends on the desired precision. It should not be hard-coded this way.
There was a problem hiding this comment.
I can add them to the rigid solver options. The LS_PARALLEL_K is similar to the BLOCK_DIM in func_hessian_direct_tiled impl, which determines the parallelism and shared memory pattern of the kernel, which is not runtime changing variable.
LS_PARALLEL_MIN_STEP is a starting low bound of the linesearch alpha search range. It is more reasonable to set it as a constant in my opinion.
There was a problem hiding this comment.
I see. Can you document them? I think it is weird to pick a constant that does not depends on genesis precision and/or linesearch tolerance. Does it make sense to use a threshold that is smaller than desired linesearch tolerance? You should document all of this.
| LS_PARALLEL_MIN_STEP = 1e-6 | ||
| LS_PARALLEL_N_REFINE = 1 # number of successive refinement passes in parallel linesearch |
There was a problem hiding this comment.
These 2 options should probably be added to the rigid solver options.
There was a problem hiding this comment.
If it does not make sense to make this parameters tunable then it is great, but you need to document why you think so.
|
To make the PRs easier for review, intend to split it into small PRs: |
2 participants
Description
Related Issue
Resolves Genesis-Embodied-AI/Genesis#
Motivation and Context
How Has This Been / Can This Be Tested?
Screenshots (if appropriate):
Checklist:
Submitting Code Changessection of CONTRIBUTING document.