materials project feature ranges based on conventional unit cells

[hasan-sayeed]

No description provided.

[sgbaird]

@hasan-sayeed thanks for doing this! A few things here: you committed your API key, so now that's available for anyone to see (despite being unlikely that someone will see it on a PR). If you're working on Colab, follow the G Drive instructions; if you're working on this locally then follow the pmg config instructions before rerunning the notebook.

I suggest using a variable name other than str, since this is a built-in Python function.

Last, to get it to pass the linting you'll need to run pre-commit on your files. See https://github.com/sparks-baird/xtal2png/runs/6952260006?check_suite_focus=true (which you get to by clicking on "details" below:

It would be nice if the final histogram image was saved in the notebook. If you're running on Colab, you need to download the notebook locally and then update the PR separately. If you're running locally, just make sure to "run all cells" from a fresh kernel and it should include the images in the nb.

Since it seems like you modified the original file instead of appending to it, leave the original file (without your changes) and rename this version to something unique, probably:

notebooks/2.0.1-materials-project-feature-ranges-primitive.ipynb

so that it gets added as a separate notebook, since we want to keep the original notebook intact.

[sgbaird]

@hasan-sayeed, I took care of the renaming (filename and str variable) and running pre-commit. See https://www.screencast.com/t/FpUjUFnOm7 (note that you'd need to do some setup per https://github.com/sparks-baird/xtal2png/blob/main/CONTRIBUTING.md#clone-the-repository for pre-commit, and you can run pre-commit run --all-files if the integration with gh desktop isn't working for you - I think I might have had to do some hack to get pre-commit working with gh desktop).

You'll still need to take care of the API key being exposed and rerunning the notebook.

[sgbaird]

@hasan-sayeed nvm about the histogram figure displaying - I'm forgetting that's just an issue with plotly and Jupyter notebooks. I'd say don't worry about that one.

[sgbaird]

@hasan-sayeed

Lower

Upper

It looks like all the values are identical to the original notebook, indicating the downloaded structures from pymatgen are by default the same as what you get with symprec=0.1, angle_tolerance=5.0, and get_primitive_standard_structure(). So, I guess the defaults are based on primitive cells. I know this is a 180 flip, but let's change this so you're getting defaults for conventional cells instead.

[sgbaird]

It would also probably be good to look at the number of sites, which you can access via len(s.sites), and there are a number of straightforward additions you'd need to make to the code.

[sgbaird]

What were the changes to the 2.0- notebook? If this wasn't intentional, go ahead and revert the changes to the notebook

[sgbaird]

@hasan-sayeed just about there; see my updates. Please re-run the latest version of the notebook so that it has the output for num_sites embedded (a change I added) into the output and commit it again.

Not sure if you had to skip the last 2 cells or if it ran OK for you. If it will run OK, please save a copy of the various HTML and PNG outputs to the xtal2png GDrive folder I shared with you since we might include these in the supporting info of a manuscript. Trying to keep the GitHub repo relatively lean for when people clone it, but the single HTML and PNG file you included here are fine. If you had to skip the last 2 cells, don't worry about those figures.

[sgbaird]

Thanks @hasan-sayeed!