Change default deepmd_version and make the screen output complete #1641
Conversation
make it to be consisten with the doc Signed-off-by: Scott Wu <[email protected]>
dlog logger defaults to broadcasting logs to root logger causing unclear output. So disable propagation to keep the output clear. Signed-off-by: Scott Wu <[email protected]>
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve modifications to the logging configuration in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Logger
participant FileHandler
participant VersionManager
User->>Logger: Initialize logging
Logger->>Logger: Set propagate to False
Logger->>FileHandler: Configure file handler for dpgen.log
Logger->>User: Log messages at WARNING level
User->>VersionManager: Call set_version()
VersionManager->>VersionManager: Update deepmd_version from "1" to "2"
VersionManager->>User: Return updated mdata
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
dpgen/__init__.py (2)
Line range hint
12-13: Consider adding a debug formatterThere's a commented-out formatter that includes more detailed information (filename, function name, line number). Consider adding this as an optional debug formatter that can be enabled when more detailed logging is needed.
You could implement this by adding a condition to use the detailed formatter when a debug flag is set. For example:
debug_mode = False # Set this to True when detailed logging is needed if debug_mode: dlogf_formatter = logging.Formatter('%(asctime)s - %(name)s - [%(filename)s:%(funcName)s - %(lineno)d ] - %(levelname)s \n %(message)s') else: dlogf_formatter = logging.Formatter("%(asctime)s - %(levelname)s : %(message)s")
Missing
deepmd_versionChange indpgen/__init__.pyThe PR objectives mention updating the default
deepmd_version, but no changes related todeepmd_versionare found indpgen/__init__.py. Please ensure that thedeepmd_versionis properly updated in this file as intended.Analysis chain
Line range hint
1-67: Query: Where is the deepmd_version change?The PR objectives mention changing the default
deepmd_version, but this change is not present in this file. Could you clarify where this change is implemented, or if it's supposed to be part of this file?
Follow-up: Correcting the search for
deepmd_versionThe previous command did not recognize the
pythonfile type. Please run the following command to search fordeepmd_versionin Python files:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for deepmd_version in the codebase rg --type python 'deepmd_version'Length of output: 64
Script:
#!/bin/bash # Search for deepmd_version in Python files rg --type py 'deepmd_version'Length of output: 3595
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- dpgen/init.py (1 hunks)
- dpgen/generator/run.py (1 hunks)
Additional comments not posted (1)
dpgen/__init__.py (1)
8-8: LGTM: Prevents duplicate logging as intendedThis change sets
dlog.propagate = False, which prevents log messages from being passed to parent loggers. This effectively addresses the PR objective of preventing duplicate output by ensuring that log messages are only written to thedpgen.logfile and not propagated to stderr or other handlers.
Signed-off-by: Scott Wu <[email protected]>
scott-5
changed the title
scott-5
changed the title
Signed-off-by: Scott Wu <[email protected]>
Signed-off-by: Scott Wu <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tests/generator/machine-local-v1.json (2)
2-2: LGTM! Consider adding a comment for clarity.The addition of
"deepmd_version": "1"is consistent with the PR objective to change the defaultdeepmd_version. This change improves the configuration file by explicitly specifying the version.Consider adding a comment above this line to explain the purpose and potential values of
deepmd_version. This would enhance maintainability and user understanding. For example:+ // Specifies the version of DeePMD-kit to use. Valid values: "0", "1", "2" "deepmd_version": "1",
Line range hint
1-47: Consider using standard JSON comments for improved readability.The JSON structure is valid and the addition of
deepmd_versionis consistent with the file structure. However, the file uses non-standard comment keys (e.g., "_comment1", "_comment2") for documentation.Consider using standard JSON comments (starting with
//) instead of comment keys. This would improve readability and maintain proper JSON structure. For example:"source_list": [ "/home/wanghan/study/deep.md/venvs/py3.6-tf1.8/bin/active" ], "envs": {}, - "_comment1": "that's All" + // that's All },Apply this change throughout the file for all comment keys.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #1641 +/- ##
==========================================
- Coverage 49.54% 49.53% -0.02%
==========================================
Files 83 83
Lines 14848 14849 +1
==========================================
- Hits 7357 7356 -1
- Misses 7491 7493 +2 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit