Fix issue where optimizable params are in model dump#801
Fix issue where optimizable params are in model dump#801rapids-bot[bot] merged 4 commits intoNVIDIA:developfrom
Conversation
Marked the agent optimizer task as complete in the README and improved LangChain callback registration to ensure fresh handlers are set in the current context. The updates reduce redundant hook registrations and enhance debugging clarity. Signed-off-by: dnandakumar-nv <dnandakumar@nvidia.com>
Marked the agent optimizer task as complete in the README and improved LangChain callback registration to ensure fresh handlers are set in the current context. The updates reduce redundant hook registrations and enhance debugging clarity. Signed-off-by: dnandakumar-nv <dnandakumar@nvidia.com>
The `optimizable_params` and `search_space` fields are now excluded from serialization. This prevents unnecessary data from being included in outputs, ensuring cleaner and more controlled serialization behavior. Signed-off-by: dnandakumar-nv <dnandakumar@nvidia.com>
WalkthroughUpdated OptimizableMixin in src/nat/data_models/optimizable.py to mark two fields—optimizable_params and search_space—as excluded from serialization by adding exclude=True to their Field definitions. No other logic or behavior changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dnandakumar-nv
added
bug
|
/ok to test ece2d84 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/nat/data_models/optimizable.py (1)
115-119: Excludesearch_spaceas well — align docs/expectations and considerrepr=False.Same rationale as above. If these fields should also be hidden from generated JSON Schema/OpenAPI, note that
exclude=Truewon’t remove them from schema; you’d need schema customization (or migrate toPrivateAttrif validation isn’t required).Apply:
- search_space: dict[str, SearchSpace] = Field( - default_factory=dict, - description="Optional search space overrides for optimizable parameters.", - exclude=True, - ) + search_space: dict[str, SearchSpace] = Field( + default_factory=dict, + description="Optional search space overrides for optimizable parameters.", + exclude=True, + repr=False, + )Please confirm:
- No consumers expect
search_spacein serialized dumps.- Docs/config schema clearly state these are not serialized by default.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/data_models/optimizable.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/data_models/optimizable.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/data_models/optimizable.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/data_models/optimizable.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/data_models/optimizable.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/data_models/optimizable.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/data_models/optimizable.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/data_models/optimizable.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/data_models/optimizable.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (1)
src/nat/data_models/optimizable.py (1)
111-114: Add repr=False to optimizable_params; confirm serialized‑config expectationsexclude=True already prevents leaking optimizable_params via model_dump/model_dump_json — add repr=False to avoid accidental logging. Verify places that persist/export configs that call model_dump() (they may need optimizable_params included or explicitly requested).
Check:
- src/nat/data_models/optimizable.py
- src/nat/profiler/parameter_optimization/parameter_optimizer.py (writes cfg_trial.model_dump() / tuned_cfg.model_dump())
- src/nat/profiler/parameter_optimization/update_helpers.py (cfg.model_dump(mode="python"))
- docs/source/reference/optimizer.md and examples/*/config_optimizer.yml
Apply:
- optimizable_params: list[str] = Field(default_factory=list, - description="List of parameters that can be optimized.", - exclude=True) + optimizable_params: list[str] = Field( + default_factory=list, + description="List of parameters that can be optimized.", + exclude=True, + repr=False, + )Quick scan:
#!/bin/bash rg -nC2 -e 'optimizable_params' -e 'model_dump\(' -e 'model_dump_json\(' -e 'yaml.dump\('
|
/merge |
2 participants
Description
Removes optimizable params and search space fields from list of serialized fields when dumping a config objects.
By Submitting this PR I confirm:
Summary by CodeRabbit