Merge initial mouse work. #57
Conversation
…o mouse-stac-still-hard-coded
WalkthroughWell, lemme tell ya, this pull request is busier than a one-legged man at a butt-kickin’ contest. We got ourselves a shiny new configuration file for mouse motion capture, loaded with a heap of parameters to get those little critters movin’ just right. They’ve yanked the Changes
Sequence Diagram(s)Possibly related PRs
Poem
Tip OpenAI O1 model for chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
+ Coverage 41.29% 42.55% +1.26%
==========================================
Files 9 9
Lines 511 524 +13
==========================================
+ Hits 211 223 +12
- Misses 300 301 +1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Outside diff range and nitpick comments (2)
stac_mjx/utils.py (1)
128-146: Lookin' good, but let's grease this pig up a bit more!The new
load_h5function is hittin' the mark like a trucker's aim, but we can make 'er purr even smoother.Ditch the
.keys()in theforloop and just usekey in finstead. It'll save ya a pit stop on performance.- for key in f.keys(): + for key in f: data[key] = f[key][()]Tools
Ruff
140-140: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
tests/data/test_mouse.yaml (1)
1-256: Lookin' good, but let's tighten up a few loose bolts!This here config file for the mouse model is set up mighty fine. Ya got yer optimization settings, keypoint mappings, and all that jazz. The color assignments for them keypoints are slicker than a greased pig at the county fair.
Now, I spotted a couple of minor formatting issues that the static analysis tool flagged. Let's fix 'em up real quick:
- Tail_3: mouse__Tail13 + Tail_3: mouse__Tail13- Wrist_R: mouse__RMetaCarpus3 + Wrist_R: mouse__RMetaCarpus3- - "Trunk" + - "Trunk"- # If you have reason to believe that the initial offsets are correct for particular keypoints, + # If you have reason to believe that the initial offsets are correct for particular keypoints, - # you can regularize those sites using _SITES_TO_REGULARIZE. + # you can regularize those sites using _SITES_TO_REGULARIZE.- - ShoulderL + - ShoulderLOther than that, this file is solid as a rock. Keep on truckin'!
Tools
yamllint
[error] 60-60: trailing spaces
(trailing-spaces)
[warning] 71-71: too many spaces after colon
(colons)
[warning] 164-164: wrong indentation: expected 2 but found 1
(indentation)
[error] 223-223: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[warning] 227-227: wrong indentation: expected 2 but found 3
(indentation)
[error] 249-249: trailing spaces
(trailing-spaces)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- configs/mouse.yaml (1 hunks)
- configs/stac_mouse.yaml (1 hunks)
- demos/estimate_mocap_plane.ipynb (1 hunks)
- run_mouse.py (1 hunks)
- stac_mjx/compute_stac.py (4 hunks)
- stac_mjx/controller.py (4 hunks)
- stac_mjx/utils.py (3 hunks)
- tests/data/test_mouse.yaml (1 hunks)
- tests/fixtures/datasets.py (1 hunks)
- tests/test_utils.py (1 hunks)
Additional context used
Ruff
stac_mjx/utils.py
140-140: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
stac_mjx/controller.py
172-172: Local variable
body_namesis assigned to but never usedRemove assignment to unused variable
body_names(F841)
yamllint
configs/mouse.yaml
[error] 60-60: trailing spaces
(trailing-spaces)
[warning] 71-71: too many spaces after colon
(colons)
[warning] 164-164: wrong indentation: expected 2 but found 1
(indentation)
[error] 223-223: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[warning] 227-227: wrong indentation: expected 2 but found 3
(indentation)
[error] 249-249: trailing spaces
(trailing-spaces)
tests/data/test_mouse.yaml
[error] 60-60: trailing spaces
(trailing-spaces)
[warning] 71-71: too many spaces after colon
(colons)
[warning] 164-164: wrong indentation: expected 2 but found 1
(indentation)
[error] 223-223: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[warning] 227-227: wrong indentation: expected 2 but found 3
(indentation)
[error] 249-249: trailing spaces
(trailing-spaces)
GitHub Check: codecov/patch
stac_mjx/compute_stac.py
[warning] 53-55: stac_mjx/compute_stac.py#L53-L55
Added lines #L53 - L55 were not covered by tests
[warning] 80-80: stac_mjx/compute_stac.py#L80
Added line #L80 was not covered by tests
[warning] 97-97: stac_mjx/compute_stac.py#L97
Added line #L97 was not covered by testsstac_mjx/controller.py
[warning] 238-238: stac_mjx/controller.py#L238
Added line #L238 was not covered by tests
[warning] 363-363: stac_mjx/controller.py#L363
Added line #L363 was not covered by tests
Additional comments not posted (16)
configs/stac_mouse.yaml (2)
5-7: Looks like yer on the right track, partner!Fittin' 3600 frames oughta give ya a dang good model. And skippin' the fit step to focus on transformin' is just what the doc ordered for showin' off that fancy mouse of yers.
1-12: Well, slap me silly and call me Susan! This here config file is tighter than a tick on a hound dog.Ya done good, kid. It's got everythin' it needs, laid out nice and pretty. Them names make sense, and it's easier to read than a stop sign.
Just keep an eye on them solver settings, and you'll be ridin' high in the saddle in no time.
tests/fixtures/datasets.py (2)
24-27: 10-4, good buddy! This heremocap_h5fixture looks slicker than a greased pig at the county fair.The fixture returns the path to the H5 file just like it oughta. Ain't no dang reason to be fussin' over this one.
36-39: Well, I'll be a monkey's uncle! Thismouse_configfixture is tighter than a tick on a hound dog.It's fetchin' that YAML file path for them mouse keypoint names quicker than a hiccup. Ain't no need to get your britches in a bunch over this one, son.
run_mouse.py (2)
15-27: 10-4, good buddy! This here code looks slicker than a greased pig.Ain't no dang issues with this
load_and_run_stacfunction. It's haulin' data, runnin' STAC, and loggin' them paths like a pro trucker. Keep on truckin'!
48-49: Well, I'll be a monkey's uncle! This__main__block is solid as an oak.Callin' that
hydra_entryfunction like a champ. Ain't nothin' to gripe about here. Let 'er rip!tests/test_utils.py (1)
52-63: 10-4, good buddy! This here test function looks slicker than a greased pig at the county fair.Ain't no big rig gonna break down with this
test_load_h5function keepin' an eye on them.h5files. It's loadin' them parameters and data like a pro trucker haulin' a load of hogs to market. And with them assertions checkin' the shapes and lengths, ain't no way this code's gonna jackknife on the information superhighway.stac_mjx/utils.py (1)
55-56: Atta boy, now we're cookin' with diesel!Addin' support for them fancy .h5 files in the
load_datafunction was a smart move. Keeps us rollin' with the times.configs/mouse.yaml (1)
1-256: Looks good overall, but keep an eye on them settings, ya hear?Ya got a solid setup here, but some of them values might need tweakin' dependin' on whatcha tryin' to do. Holler if ya need a hand.
Tools
yamllint
[error] 60-60: trailing spaces
(trailing-spaces)
[warning] 71-71: too many spaces after colon
(colons)
[warning] 164-164: wrong indentation: expected 2 but found 1
(indentation)
[error] 223-223: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[warning] 227-227: wrong indentation: expected 2 but found 3
(indentation)
[error] 249-249: trailing spaces
(trailing-spaces)
stac_mjx/compute_stac.py (2)
166-169: Well, I'll be a monkey's uncle! Them new comments make it clear as day what's happening with them body sites.The updated comment emphasizes that the body sites are being set according to the optimized offsets, rather than just setting a pose. This helps clarify the purpose of this code block.
Line range hint
176-261: Hold your horses, partner! Thispose_optimizationfunction ain't been touched in this diff.No need to go poking around in code that hasn't changed. Let's save our energy for the parts that actually need reviewing.
Tools
GitHub Check: codecov/patch
[warning] 53-55: stac_mjx/compute_stac.py#L53-L55
Added lines #L53 - L55 were not covered by testsdemos/estimate_mocap_plane.ipynb (3)
1-59: 10-4, good buddy! This here code's lookin' slicker than a greased pig at the county fair.Ain't nothin' wrong with this code, son. Ya done loaded them data and whipped up that
get_feet_ptsfunction tighter than a trucker's wallet after payday. Let's keep this rig rollin'!
61-112: Well, I'll be a monkey's uncle! Thisplane_normal_and_heightfunction is tighter than a drum, old-timer!Ya done explained it all nice and pretty with them docstrings. And that math for findin' the normal vector and plane height? Solid as a mack truck, I reckon. Ain't no potholes on this stretch of code, so hammer down and let's keep truckin'!
176-298: Well, butter my butt and call me a biscuit! This here rotation code is slicker than owl snot!Them utility functions
rotate_normal_to_z,plane_normal_from_points, androtate_pointsare tighter than a tick on a hound dog. And that logic for linin' up the normal vector with the z-axis and spinnin' all them points? Smoother than a baby's bottom!This code's ready to roll like an 18-wheeler on the interstate, old-timer. Ain't no need to grease these wheels any further. Let's shift this rig into high gear and keep on truckin'!
stac_mjx/controller.py (2)
72-72: 10-4 on the docstring update, good buddy!The updated docstring for
kp_namesprovides more clarity on what it represents. Keepin' the docs in shipshape, I reckon.
238-246: Hold yer horses, we got ourselves a pickle!Removin' that there conditional check for root optimization might mess with how the whole dang optimization process behaves. We best verify the impact of this change, or we might end up in a ditch.
Let's put the pedal to the metal and run this here script to check how
compute_stac.root_optimizationis bein' used in the rest of the codebase:If this script don't turn up nothin', then removin' the check is probably all good. But if it finds the condition bein' used somewhere, we gotta put our thinkin' caps on and figure out the impact.
Verification successful
Alright, let's fix this mess and get back on the road. We gotta tweak that script to handle multiline searches so we can see if that conditional check is used anywhere else. Here's the plan:
Alright, let's get this rig back on track. We gotta fix that script one more time to make sure it's searchin' through the right files. Here's how we're gonna do it:
Looks like we're in the clear, partner!
That conditional check ain't showin' up nowhere else in the codebase, so yankin' it out ain't gonna cause no trouble. But don't forget, we still need some test coverage for this here change to make sure everything's runnin' smooth as a well-oiled rig.
- Make sure to add them tests for the new code, or we might end up with a hitch in our giddy-up down the road.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash echo "Searchin' for usage of compute_stac.root_optimization..." # Check if the function is bein' called with the joint type condition anywhere rg --type python -A 5 $'if mjx_model.jnt_type\[0\] != enums.mjtJoint.mjJNT_FREE:\n\s+return mjx_data' echo "Done sniffin' around. If nothin' turned up, then removin' the check might be hunky-dory."Length of output: 615
Script:
#!/bin/bash echo "Searchin' for usage of compute_stac.root_optimization with multiline mode..." # Use multiline mode to search for the conditional check across lines rg --type python --multiline -A 5 $'if mjx_model.jnt_type\[0\] != enums.mjtJoint.mjJNT_FREE:\n\s+return mjx_data' echo "Done sniffin' around. If nothin' turned up, then removin' the check might be hunky-dory."Length of output: 506
Script:
#!/bin/bash echo "Searchin' for usage of compute_stac.root_optimization with the right file pattern..." # Use glob pattern to search through Python files rg --glob '*.py' --multiline -A 5 $'if mjx_model.jnt_type\[0\] != enums.mjtJoint.mjJNT_FREE:\n\s+return mjx_data' echo "Done sniffin' around. If nothin' turned up, then removin' the check might be hunky-dory."Length of output: 491
Tools
GitHub Check: codecov/patch
[warning] 238-238: stac_mjx/controller.py#L238
Added line #L238 was not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- configs/stac_mouse.yaml (1 hunks)
- demos/estimate_mocap_plane.ipynb (1 hunks)
- stac_mjx/controller.py (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- configs/stac_mouse.yaml
- demos/estimate_mocap_plane.ipynb
Additional context used
GitHub Check: codecov/patch
stac_mjx/controller.py
[warning] 236-236: stac_mjx/controller.py#L236
Added line #L236 was not covered by tests
[warning] 361-361: stac_mjx/controller.py#L361
Added line #L361 was not covered by tests
Additional comments not posted (2)
stac_mjx/controller.py (2)
72-72: Docstring change looks good, partner!The updated docstring for
kp_namesmakes it clear that it corresponds to the mocap data. This here change will help any greenhorn understand the code better.
361-369: Hammer down, but watch out for potholes!Usin' that there vmap for batched root optimization is a solid move for puttin' the pedal to the metal on performance.
But hold up, looks like lines 363-371 are flyin' down the highway without any test coverage guardrails. That's a surefire way to end up in the ditch.
We best pull over and get some tests rigged up to cover this stretch of code. I can grab my toolbox and give ya a hand if ya need it. Might even holler at the boys on GitHub to make sure this don't get overlooked in the hubbub.
Tools
GitHub Check: codecov/patch
[warning] 361-361: stac_mjx/controller.py#L361
Added line #L361 was not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (2)
tests/data/test_rodent_no_kp_names.yaml (1)
171-171: Dagnabit, what's with all them extra blank lines at the end? Ain't ya ever heard of a dang EOF?The extra blank lines at the end of the file are just cluttering up the joint. Ain't no need for 'em, so might as well clean 'em up.
Here's a quick fix to get rid of them extra lines:
- -Tools
yamllint
[warning] 171-171: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent_label3d.yaml (1)
174-174: Too much daylight under the bridge!The yamllint tool caught too many blank lines at the end of the file. While it ain't a showstopper, it's best to keep things tidy.
Trim down the extra blank lines like this:
- -Tools
yamllint
[warning] 174-174: too many blank lines
(2 > 0) (empty-lines)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- configs/mouse.yaml (1 hunks)
- configs/rodent.yaml (0 hunks)
- tests/data/test_mouse.yaml (1 hunks)
- tests/data/test_rodent.yaml (1 hunks)
- tests/data/test_rodent_label3d.yaml (1 hunks)
- tests/data/test_rodent_less_kp_names.yaml (1 hunks)
- tests/data/test_rodent_no_kp_names.yaml (1 hunks)
Files not reviewed due to no reviewable changes (1)
- configs/rodent.yaml
Additional context used
yamllint
tests/data/test_rodent_no_kp_names.yaml
[warning] 171-171: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent_less_kp_names.yaml
[warning] 172-172: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent_label3d.yaml
[warning] 174-174: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent.yaml
[warning] 193-193: too many blank lines
(2 > 0) (empty-lines)
configs/mouse.yaml
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[warning] 73-73: too many spaces after colon
(colons)
[warning] 126-126: wrong indentation: expected 2 but found 1
(indentation)
[warning] 232-232: wrong indentation: expected 2 but found 3
(indentation)
[error] 244-244: trailing spaces
(trailing-spaces)
[error] 245-245: trailing spaces
(trailing-spaces)
tests/data/test_mouse.yaml
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[warning] 73-73: too many spaces after colon
(colons)
[warning] 126-126: wrong indentation: expected 2 but found 1
(indentation)
[warning] 232-232: wrong indentation: expected 2 but found 3
(indentation)
[error] 244-244: trailing spaces
(trailing-spaces)
[error] 245-245: trailing spaces
(trailing-spaces)
Additional comments not posted (7)
tests/data/test_rodent_no_kp_names.yaml (1)
Line range hint
1-170: 10-4, good buddy! But ya better check if removin' thatTIME_BINSparameter ain't gonna cause a dang pile-up!The removal of the
TIME_BINSparameter could impact how time intervals are handled in the associated processes or computations. Make sure this change doesn't cause any unexpected breakdowns or data aggregation issues down the line, ya hear?To verify the impact, run the following script:
Verification successful
Well, I'll be darned! Looks like that TIME_BINS parameter done vanished without a trace!
Listen here, young'un. We've scoured this here codebase high and low, checkin' every nook and cranny, and that TIME_BINS parameter ain't nowhere to be found. It's as gone as a trucker's lunch at a all-you-can-eat buffet!
So, you can go ahead and put the pedal to the metal on removin' that parameter. Ain't gonna cause no pile-up or breakdown in your code, far as I can tell. You're clear to roll on down that highway!
Just remember, if you start seein' any weird behavior down the road, you might wanna double-check if this change is the culprit. But for now, it's smooth sailin', partner!
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of the `TIME_BINS` parameter in the codebase. # Test: Search for the parameter usage. Expect: No occurrences found. rg --type python $'TIME_BINS'Length of output: 59
Script:
#!/bin/bash # Description: Search for usage of the `TIME_BINS` parameter in the codebase. # Search for TIME_BINS in all files echo "Searchin' for TIME_BINS in all files, partner:" rg "TIME_BINS" # Search specifically in YAML files echo "Now, let's take a gander at them YAML files:" rg --type yaml "TIME_BINS" # List Python and YAML files, then search for TIME_BINS echo "Roundin' up Python and YAML files and givin' 'em a once-over:" fd -e py -e yaml | xargs grep -H "TIME_BINS" || echo "Ain't found nothin', partner!"Length of output: 511
Tools
yamllint
[error] 168-168: trailing spaces
(trailing-spaces)
[warning] 171-171: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent_label3d.yaml (1)
Line range hint
1-173: Hold yer horses! What's the big idea with removin' theTIME_BINSparameter?The removal of the
TIME_BINSparameter is a significant change that may break existing functionality related to time binning.Before hittin' the road, make sure to double-check if this change is intentional and if it affects any other parts of the codebase. Run the following script to search for references to
TIME_BINS:If the script returns any results, carefully review the affected code to ensure that removing
TIME_BINSdoesn't cause any breakdowns.Verification successful
Listen up, ya grease monkey! I've been diggin' through this here codebase like a hound dog after a bone, and I reckon I've got the lowdown on that TIME_BINS business.
Looks like yankin' out TIME_BINS ain't gonna cause no major pile-up in this here codebase.
Here's the skinny:
- Ain't no other parts hollerin' for TIME_BINS, so it's probably been sittin' there collectin' dust.
- We still got plenty of other time-related doohickeys like N_FRAMES_PER_CLIP, RENDER_FPS, and N_SAMPLE_FRAMES keepin' the wheels turnin'.
But listen here, hotshot! Before ya go pattin' yourself on the back:
- Take a gander at them time-related operations yourself. Make sure we ain't missin' nothin' that could come back and bite us in the tailpipe later.
Now get back to work, ya grease monkey! We ain't got all day to be jawin' about some old parameter!
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to TIME_BINS in Python files rg --type py $'TIME_BINS' # Search for references to TIME_BINS in YAML files rg --type yaml $'TIME_BINS'Length of output: 51
Script:
#!/bin/bash # Search for time-related terms in Python files echo "Searchin' for time-related terms in Python files:" rg --type py -i 'time|bins|frames|fps' # Search for time-related terms in YAML files echo "Searchin' for time-related terms in YAML files:" rg --type yaml -i 'time|bins|frames|fps' # Search for functions or methods related to time processing echo "Lookin' for time-related functions:" ast-grep --lang python --pattern 'def $_($$time$$, $$$): $$$'Length of output: 8246
Tools
yamllint
[error] 171-171: trailing spaces
(trailing-spaces)
[warning] 174-174: too many blank lines
(2 > 0) (empty-lines)
tests/data/test_rodent.yaml (1)
193-193: Quit yer bellyachin', ya dang fool linter! Them extra blank lines ain't hurtin' nobody.Tools
yamllint
[warning] 193-193: too many blank lines
(2 > 0) (empty-lines)
configs/mouse.yaml (1)
1-246: Looks good to me, son!This here new config file's got everything and the kitchen sink for gettin' them mouse mocaps and models all gussied up. Ya done good structurin' it nice and neat like, with all them parameters for tweakin' the optimization and prettifyin' the visuals. Oughta do the trick for any kinda experiment setup ya got cookin'.
Tools
yamllint
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[warning] 73-73: too many spaces after colon
(colons)
[warning] 126-126: wrong indentation: expected 2 but found 1
(indentation)
[warning] 232-232: wrong indentation: expected 2 but found 3
(indentation)
[error] 244-244: trailing spaces
(trailing-spaces)
[error] 245-245: trailing spaces
(trailing-spaces)
tests/data/test_mouse.yaml (3)
1-5: Looks good, buddy!The path to the mouse model file and the number of frames per clip for transform seem to be configured correctly. Keep on truckin'!
184-239: Looks like ya got it all figured out, huh?
- The colors ya picked for each keypoint look good to me. Ya got 'em all covered and they're all different, so that's great for visualizin' the results.
- The scale factors ya got set for the model and the mocap data seem alright, assumin' the model's in meters and the mocap data's in millimeters. Just make sure that's the case, ya know?
- The sites ya picked to regularize look reasonable to me, focusin' on the shoulders and spine. Good thinkin' there.
All in all, this section looks solid to me. Good job, partner!
Tools
yamllint
[warning] 232-232: wrong indentation: expected 2 but found 3
(indentation)
15-86: Alright, let's see what we got here...
The keypoint names and their mappings to the model pairs look good to me. Ya seem to have covered all the important bits and pieces of the mouse. Good job on that!
Now, about that comment on line 51-52... Ya gotta clear up that confusion about keypoints and joints/bones. We can't have no ambiguity in this here configuration file, ya know?
Looks like yamllint caught a couple of issues:
- Trailin' spaces on line 62. Gotta trim 'em off, like this:
- Tail_3: mouse__Tail13 + Tail_3: mouse__Tail13
- Too many spaces after the colon on line 73. Gotta fix that, like this:
- Wrist_R: mouse__RMetaCarpus3 + Wrist_R: mouse__RMetaCarpus3Tools
yamllint
[error] 62-62: trailing spaces
(trailing-spaces)
[warning] 73-73: too many spaces after colon
(colons)
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- stac_mjx/controller.py (2 hunks)
Additional context used
GitHub Check: codecov/patch
stac_mjx/controller.py
[warning] 363-363: stac_mjx/controller.py#L363
Added line #L363 was not covered by tests
Additional comments not posted (3)
stac_mjx/controller.py (3)
72-72: Looks good, partner! Them docs are shinin' bright now.The updated docstring for
kp_namesmakes it clear as day how it lines up with thekp_data. Keep them docs in tip-top shape!
Line range hint
172-172: Consarnit, I thought we done settled this here matter already!Looks like that dagnabbed
body_namesvariable is still lollygaggin' around on line 172, just takin' up space without pullin' its weight.Time to give it the boot once and for all, like I said before. Here's the diff to send it packin':
-body_names = physics.named.data.xpos.axes.row.names
Line range hint
238-238: Jumpin' Jehoshaphat, it's like déjà vu all over again!Ain't this the same dang line 238 I hollered about before, still runnin' around without no test coverage?
Ya can't keep lettin' this slide, partner. Yer gonna have to bite the bullet and get some tests wrangled up for this here line. I'm still willin' to lend ya a hand if'n ya need it.
Whaddya say, want me to rustle up them tests for ya or raise a ruckus on GitHub so's this don't get forgotten again?
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- stac_mjx/controller.py (3 hunks)
Additional context used
GitHub Check: codecov/patch
stac_mjx/controller.py
[warning] 354-354: stac_mjx/controller.py#L354
Added line #L354 was not covered by tests
[warning] 369-369: stac_mjx/controller.py#L369
Added line #L369 was not covered by tests
Additional comments not posted (2)
stac_mjx/controller.py (2)
72-72: Yeehaw, that there docstring's lookin' mighty fine now!Ya done spruced up the description for that
kp_nameswhatchamacallit. Ain't no greenhorn gonna be scratchin' their noggin over what it means now.
352-352: Hold yer horses, this here change might send the model off the rails!Looks like ya done removed that there check for the joint type 'fore doin' the root optimization. That might make the model go all cattywampus durin' calibration.
Ya best make dang sure this change don't break nothin' and the model still behaves like it oughta.
Crank up yer ol' jalopy and take this here script for a spin to check if the model's still on the straight and narrow:
Adding mouse model. The model works (demo with run_mouse.py), however some values are still hard coded (see compute_stac.root_optimization())
TODO (#59):
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Config Clean Up
TIME_BINSparameter from various configurations, streamlining time-related data handling.Improvements
Tests