Add `calc_displacements_ellipsoid` function #87

kavanase · 2024-08-20T16:26:00Z

No description provided.

kavanase · 2024-08-20T16:28:05Z

coderabbitai · 2024-08-20T16:28:09Z

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2024-08-20T16:28:12Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The recent changes to the doped/utils/displacements.py module significantly improve its functionality for analyzing defect-related displacements in crystal structures. A new function, calc_displacements_ellipsoid, has been introduced to fit ellipsoids to displacements, enhancing visualization. Additionally, the existing calc_site_displacements function has been refined for clarity, and import statements have been organized. These enhancements not only broaden analytical capabilities but also streamline the code for better readability and maintainability.

Changes

File Path	Change Summary
`doped/utils/displacements.py`	- Added `calc_displacements_ellipsoid` to calculate and visualize defect displacements.
	- Refined `calc_site_displacements`, improving logic and structure by repositioning `_get_bulk_struct_with_defect`.
	- Updated import statements and removed commented code for clarity and maintainability.

Poem

Hop, hop, in the code we delight,
New ellipsoids shimmering bright!
With clarity now, our paths align,
Defect displacements, oh what a design!
A rabbit's cheer for the code we mend,
In the crystal world, new journeys ascend! 🐇✨

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-- I pushed a fix in commit <commit_id>, please review it.
-- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
-- @coderabbitai generate unit testing code for this file.
-- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
-- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
-- @coderabbitai read src/utils.ts and generate unit testing code.
-- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (9)

doped/utils/displacements.py (9)
604-604: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- marker=dict(color="black", size=3),
+ marker={"color": "black", "size": 3},
Tools

Ruff

604-604: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

609-611: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- scene=dict(
-     aspectmode="data",
- ),
+ scene={"aspectmode": "data"},
Tools

Ruff

609-611: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

638-638: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- line=dict(color="black", width=2),
+ line={"color": "black", "width": 2},
Tools

Ruff

638-638: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

744-753: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- marker=dict(
-     size=10,
-     opacity=0.5,
-     color=anisotropy_info_df["the_longest_radius"],  # Set color according to column "a"
-     colorscale="rainbow",
-     colorbar=dict(
-         title="The longest radius of ellipsoid",
-         titleside="right",
-     ),
- ),
+ marker={
+     "size": 10,
+     "opacity": 0.5,
+     "color": anisotropy_info_df["the_longest_radius"],  # Set color according to column "a"
+     "colorscale": "rainbow",
+     "colorbar": {
+         "title": "The longest radius of ellipsoid",
+         "titleside": "right",
+     },
+ },
Tools

Ruff

744-753: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

749-752: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

761-761: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- line=dict(color="black", dash="dash"),
+ line={"color": "black", "dash": "dash"},
Tools

Ruff

761-761: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

771-771: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- title_font=dict(size=10),
+ title_font={"size": 10},
Tools

Ruff

771-771: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

772-772: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- tickfont=dict(size=12),
+ tickfont={"size": 12},
Tools

Ruff

772-772: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

782-782: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- title_font=dict(size=10),
+ title_font={"size": 10},
Tools

Ruff

782-782: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

783-783: Optimize dictionary creation.

Rewrite the dict call as a literal for better performance and readability.
- tickfont=dict(size=12),
+ tickfont={"size": 12},
Tools

Ruff

783-783: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b3c674d and ca0d8c1.

Files selected for processing (1)

doped/utils/displacements.py (3 hunks)

Additional context used

Ruff

doped/utils/displacements.py

604-604: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

609-611: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

638-638: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

744-753: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

749-752: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

761-761: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

771-771: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

772-772: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

782-782: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

783-783: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

Additional comments not posted (2)

doped/utils/displacements.py (2)

35-35: LGTM!

The changes to calc_site_displacements improve clarity and modularity. The function is well-structured and handles edge cases appropriately.

985-985: LGTM!

The repositioning of _get_bulk_struct_with_defect improves the modularity of the code. The function is well-documented and handles different defect types appropriately.

kavanase · 2024-08-20T16:34:20Z

@teruya7 this looks very nice, thank you very much for adding to the code! 🙌

Some small things on the checklist to get this merged:

Would you mind adding some quick test cases, similar to those added for the original calc_site_displacements functions in test_displacements.py?
- Likewise, adding a short example (probably using one of these test cases) to the advanced analysis tutorial would be nice to illustrate this feature!
(Lower priority): It would be nice if, similar to plot_site_displacements, using plotly instead of matplotlib was an option rather than a requirement. Would you be able to try implementing both backends?

kavanase · 2024-08-20T16:36:23Z

Also following the CodeRabbit suggestions above; could you change the dict() calls to literal dicts ({...}) as suggested? This is the preferred formatting if you use the doped pre-commit

teruya7 · 2024-08-21T11:14:39Z

@kavanase
Thank you for your kind comments.
I will modify the points you have mentioned.

kavanase · 2024-08-23T16:09:44Z

@teruya7 that was quick! 🙌
I have given you write access to doped now, so would be easiest to push directly to this PR for making this as efficient as possible.
Can you make sure to pull this branch as is first before making any other updates, to avoid merge conflicts? (There were a good few with the last branch so this will avoid some headaches).

kavanase · 2024-08-23T16:50:16Z

Added some other fixes and cleanup there. One issue, for the cell axes in the plotly outputs, there's a small issue where it seems to be getting them a bit wrong:

(From the tutorial notebook plot)

I think this is probably due to my refactoring of that part of the code to try make it a bit more succinct. Would you be able to find the issue with this and fix it? 🙏

kavanase

Added two code requests there!

Once done, I think this is all ready to be merged 😃
Thanks again @teruya7!

tests/test_displacements.py

doped/utils/displacements.py

kavanase · 2024-08-23T16:56:03Z

I also updated the functions to return the Figure objects if generated btw, which is useful so users can further customise if they want, and for testing.

teruya7 · 2024-08-25T10:44:46Z

Thank you for checking!
I will modify codes.

kavanase · 2024-09-16T17:16:06Z

Hi @teruya7!
Just checking in, we're hoping to release a new major version of doped in the coming weeks, and would be nice to have this functionality included. Would you have time to work on the remaining todos from the code review above over the next while? 🙏 Thanks again for these additions!

teruya7 · 2024-09-16T22:11:32Z

@kavanase
I have been busy lately and have not been able to update the code. I will update by tomorrow.

teruya7 · 2024-09-17T01:39:38Z

@kavanase
I'm sorry for the delay.
I have corrected the things you pointed out.

kavanase · 2024-09-17T14:52:12Z

@teruya7, no problem at all!
Thanks for your work on this. Can you push these updates? (There are no new commits showing on this PR)

kavanase · 2024-09-17T15:05:20Z

@teruya7 there were some merge conflicts with the develop branch, so I have fixed these now and pushed to this PR. I think you will need to pull the current PR with git pull --rebase, and then git push your updates. Thanks!

teruya7 · 2024-09-19T13:44:59Z

@kavanase
I pulled the current PR and then pushed my update. Could you check whether the merge conflict is solved or not?

kavanase · 2024-09-19T21:24:09Z

Thanks for your work on this @teruya7!
I've gone through and cleaned up the code a bit and added some more tests. Merging to develop now!
This will be included in the next doped release, which should be in the coming weeks 🤞

teruya7 and others added 3 commits August 12, 2024 14:13

added print function

55974b7

added calc_displacements_ellipsoid function

74a523d

Formatting and cleanup

ca0d8c1

kavanase mentioned this pull request Aug 20, 2024

Add calc_displacements_ellipsoid function #86

Closed

kavanase changed the title ~~displacements2~~ Add calc_displacements_ellipsoid function Aug 20, 2024

SMTG-Bham deleted a comment from coderabbitai bot Aug 20, 2024

coderabbitai bot reviewed Aug 20, 2024

View reviewed changes

Merge branch 'develop' into displacements2

3e1f8e3

teruya7 and others added 6 commits August 23, 2024 11:33

modify calc_displacements_ellipsoid function

7c10c8b

modify calc_displacements_ellipsoid 2

7457f83

Merge

03e15cc

Cleanup imports

fdf32cc

Update default options for consistency

70938a5

Update tutorial examples

f8296d8

kavanase added the enhancement New feature or request label Aug 23, 2024

Add style_file option, return figures, cleanup etc

534d738

kavanase commented Aug 23, 2024

View reviewed changes

tests/test_displacements.py Show resolved Hide resolved

doped/utils/displacements.py Outdated Show resolved Hide resolved

teruya7 added 2 commits September 2, 2024 08:46

Refactor _plot_lattice function

690ff4f

add test_calc_site_displacements for substitution and interstitial

6d60c3c

Merge develop, and update tests

6adf382

teruya7 added 4 commits September 18, 2024 23:30

Modify duplicates

1d4e4a0

add image comparison tests

63a0b26

modify duplicate

29be195

modify test_displacement

86bbf54

kavanase added 4 commits September 19, 2024 16:53

Refactor displacements code, make more efficient, reduce redundancy etc

6d259a3

Update tests

1192ead

Add ellipsoid plotting tests for interstitials and substitutions

88fda80

Update advanced analysis notebook

d53b0e4

kavanase self-assigned this Sep 19, 2024

kavanase merged commit cb7df36 into develop Sep 19, 2024
2 of 9 checks passed

kavanase deleted the displacements2 branch September 19, 2024 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `calc_displacements_ellipsoid` function #87

Add `calc_displacements_ellipsoid` function #87

kavanase commented Aug 20, 2024

kavanase commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

kavanase commented Aug 20, 2024

kavanase commented Aug 20, 2024

teruya7 commented Aug 21, 2024

kavanase commented Aug 23, 2024

kavanase commented Aug 23, 2024

kavanase left a comment

kavanase commented Aug 23, 2024

teruya7 commented Aug 25, 2024

kavanase commented Sep 16, 2024

teruya7 commented Sep 16, 2024

teruya7 commented Sep 17, 2024

kavanase commented Sep 17, 2024

kavanase commented Sep 17, 2024

teruya7 commented Sep 19, 2024

kavanase commented Sep 19, 2024

Add calc_displacements_ellipsoid function #87

Add calc_displacements_ellipsoid function #87

Conversation

kavanase commented Aug 20, 2024

kavanase commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024 • edited Loading

Review skipped

Walkthrough

Changes

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

kavanase commented Aug 20, 2024

kavanase commented Aug 20, 2024

teruya7 commented Aug 21, 2024

kavanase commented Aug 23, 2024

kavanase commented Aug 23, 2024

kavanase left a comment

Choose a reason for hiding this comment

kavanase commented Aug 23, 2024

teruya7 commented Aug 25, 2024

kavanase commented Sep 16, 2024

teruya7 commented Sep 16, 2024

teruya7 commented Sep 17, 2024

kavanase commented Sep 17, 2024

kavanase commented Sep 17, 2024

teruya7 commented Sep 19, 2024

kavanase commented Sep 19, 2024

Add `calc_displacements_ellipsoid` function #87

Add `calc_displacements_ellipsoid` function #87

coderabbitai bot commented Aug 20, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)