theochem / gbasis

Python library for analytical evaluation and integration of Gaussian-type basis functions and related quantities.
http://gbasis.qcdevs.org/
GNU Lesser General Public License v3.0
40 stars 22 forks source link

Basis set tutorial rebased #158

Closed marco-2023 closed 10 months ago

marco-2023 commented 10 months ago

This is #145 with revisions and rebased on master (so the examples run without problem).

This PR goes through the examples of making basis set instances and the different ways to do so.

Checklist

Type of Changes

Type
:scroll: Docs

Related

FarnazH commented 10 months ago

@marco-2023, alternatively, you could have force-pushed to @maximilianvz's branch. Does this mean that PR #145 can be closed? I will review this PR soon.

marco-2023 commented 10 months ago

@FarnazH @maximilianvz I went through the notebook one last time and I found errors in the last two sections (IODATA and pySCF) I fixed them and I also changed some variable names to distinguish between a basis and a shell. Sorry about that.

maximilianvz commented 10 months ago

@marco-2023, could you please add me as a collaborator to your fork of gbasis? I have some minor formatting changes to push to this branch.

Separately, I'm going to ask you to take a look at section 1.2.3 again. The highlighted section of the text here is a little unclear to me:

Screenshot 2024-01-12 at 5 56 15 PM

In this case, when you pass a list of coord_types to make_contractions, if hydrogen 1 contributes 3 shells to basis and hydrogen 2 contributes 3 shells to basis, the coord_types list should be of the form [coord type for first shell of hydrogen 1, coord_type for second shell of hydrogen 1, coord_type for third shell of hydrogen 1, coord type for first shell of hydrogen 2, coord_type for second shell of hydrogen 2, coord_type for third shell of hydrogen 2] (the first 3 coordinate types correspond to hydrogen 1 shells and the last 3 to hydrogen 2 shells).

In this example, did you intend to have the shells for hydrogen 1 all in Cartesian coordinates and all the shells for hydrogen 2 in spherical coordinates? If so, the list should be something like ['c', 'c', 'c', 'p', 'p', 'p'] (or equivalent).

FarnazH commented 10 months ago

Thanks @marco-2023, I went through the notebook and made some changes. I will share it tomorrow after finishing sketching the corresponding section of the paper.

marco-2023 commented 10 months ago

Hi @maximilianvz thanks for pointing this out. At the beginning that was my idea but this later changed to depicting how you could arbitrarily set the types of contracted shell types (I forgot to update the markdown text). Can you please fix this?

PD: If you think is better to use cartesian shells for one atom and spherical for the other please feel free make the changes.

maximilianvz commented 10 months ago

@marco-2023, I think it would be clearer to point out to users that the length and ordering of the coord_types list matters with respect to the specified list of atoms. I'll change it to Cartesian for hydrogen 1 and spherical for hydrogen 2, and clarify this in the highlighted text from the screenshot I sent.

maximilianvz commented 10 months ago

I just updated the example as per my last comment. @FarnazH or @marco-2023, I ask that you inspect my changes and ensure that I haven't made the wording above the code cell in Section 1.2.3 too lengthy. Thanks.

FarnazH commented 10 months ago

@marco-2023 and @maximilianvz: some changes were missing resulting in an invalid notebook, so I just pushed some fixes to master; see 97713cd. For the latest notebook see: https://github.com/theochem/gbasis/blob/master/notebooks/tutorial/Tutorial%201%20-%20Basis%20Sets.ipynb