Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for toml input files and updated hubbard example notebook #76

Merged
merged 21 commits into from
Apr 19, 2024

Conversation

adsrichards
Copy link
Member

Rick and I have discussed the possibility of supporting input files in toml format. I have just created a basic script implementing toml input support called toml_to_moha.py within a new directory called scripts, and an example toml input file called hubbard.toml in the examples/toml directory to demonstrate its usage.

The script is called by using
py toml_to_moha.py /path/to/input_file.toml
The input_file.toml is then read and a model hamiltonian object is created according to the defined options within input_file.toml.

For example, using
py toml_to_moha.py ../examples/toml/hubbard.toml
from within the scripts directory calculates a HamHub object in the same way as in the hubbard example ipynb notebook.

In this example, the control parameters defined in hubbard.toml are

[control]
prefix = "hubbard"
outdir = "./fcidump"
save_intergrals = true
integral_format = "fcidump"

so that the script creates a new directory specified by outdir = "./fcidump", and the integrals are saved to fcidump format as "hubbard.out" within the outdir.

Additionally, I have cleaned up the hubbard example notebook which previously had some incorrectly calculated FCI energies.

@RichRick1 RichRick1 self-requested a review April 10, 2024 16:38
@RichRick1
Copy link
Collaborator

That looks great to me! If I understand it correctly, it can support right now only Hubbard model hamiltonian? Can we add support for Huckel, PPP, and Heisenberg models?

The HamHuck constructor calls the HamHub constructor with invalid parameters causing an error. Setting u_onsite=None is more appropriate. Also, I have fixed the typo "Huckle" -> "Huckel".
@adsrichards
Copy link
Member Author

I have just included support for all of the implemented hamiltonians in the toml_to_ham.py file including Huckel, PPP, Hubbard, Heisenberg, Ising, and RG. I have also made an example toml file for each in examples/toml. This should be up to date and working with everything now.

Copy link
Collaborator

@RichRick1 RichRick1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

Overall there are few minor changes that needs to be done:

  1. There is a typo in the toml file to save integrals
  2. We need to support saving .npz file through the toml file
  3. When generating output file it's a good idea to use os.path.join() function, so it will take care of format of the path, that may vary for different os

#-- Fermion models --#
# PPP
if data["model"]["hamiltonian"] == "ppp":
gamma = gamma0 * np.eye(norb)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to clearly document that input of PPP hamiltonian supports only input of the diagonal term $\gamma_{pq} = \gamma_0\cdot \delta_{pq}$

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the line:
"Only diagonal form of gamma matrix for PPP model is supported:
gamma_pq = gamma0 * delta_pq."

to the description of build_moha_moltype_1d to address this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just realized, that 2 body term in our definition is

$$\frac{1}{2} \sum_{p \neq q} \gamma_{p q}\left(\hat{n}{p \alpha}+\hat{n}{p \beta}-Q_p\right)\left(\hat{n}{q \alpha}+\hat{n}{q \beta}-Q_q\right)$$

so that p!= q. It seems like $\gamma_0$ is inside $U_p$
I don't think we even need this parameter to be specified.

# save integrals to outdir if specified in toml_file
if not exists(data["control"]["outdir"]):
makedirs(data["control"]["outdir"], exist_ok=True)
out_file = data["control"]["outdir"] + "/" + data["control"]["prefix"] + ".out"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to use os.path.join() function to generate the out_file. Join symbol / can vary between different os: windows use "", while MacOS and Linux use "/"

Copy link
Member Author

@adsrichards adsrichards Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have replaced this line with:
out_file = join(data["control"]["outdir"], data["control"]["prefix"])
and added 'from os.path import join' to the imports

out_file = data["control"]["outdir"] + "/" + data["control"]["prefix"] + ".out"

# save output
if data["control"]["integral_format"] == "fcidump":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently added saving hamiltonian as .npz output file. It would be great if we can support it too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made two changes to the save output. As you suggested, I include the option to specify save as npz as an option in the toml input, which calls ham.savez(out_file). I have also changed the file extension for fcidump files to be .fcidump, so that not every output file has a .out file extension.

@adsrichards
Copy link
Member Author

Thank you for noticing the typos and comment formatting. Those issues should be fixed now. I have also added the option to support saving npz, and I use os.path.join() instead of + "/" +.

@adsrichards adsrichards requested a review from RichRick1 April 18, 2024 15:49
bc = "open"
norb = 0
nelec = 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you think we can/should add default symmetry here? Make it 4 as default

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symmetry has been included in the defaults.toml file and is set to 4

#-- Fermion models --#
# PPP
if data["model"]["hamiltonian"] == "ppp":
gamma = gamma0 * np.eye(norb)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just realized, that 2 body term in our definition is

$$\frac{1}{2} \sum_{p \neq q} \gamma_{p q}\left(\hat{n}{p \alpha}+\hat{n}{p \beta}-Q_p\right)\left(\hat{n}{q \alpha}+\hat{n}{q \beta}-Q_q\right)$$

so that p!= q. It seems like $\gamma_0$ is inside $U_p$
I don't think we even need this parameter to be specified.


# set Carbon params as default in Huckel
if param_type == "model" and param_type == "huckel":
pass
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add defaults for the Huckel

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaults specific to Huckel are now set in the set_defaults function

@adsrichards
Copy link
Member Author

The parameters for the PPP model should be set correctly now, two-electron integral symmetry is set to 4 by default and included in defaults.toml, and parameters specific to the Huckel model are set in the set_defaults function.

@adsrichards adsrichards requested a review from RichRick1 April 18, 2024 20:24
@adsrichards
Copy link
Member Author

I have changed the implementation slightly. The toml_to_ham is changed from taking a string for the name of toml_file to a dict containing model data. This will be much more useful because we can generate the dict using other methods (e.g. ChatGPT), and the dict used in toml_to_ham.py is just loaded in one line. All of the methods in toml_to_ham.py can now be imported and used in the upcoming chatgpt_to_moha.py script.

Copy link
Collaborator

@RichRick1 RichRick1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

@RichRick1 RichRick1 merged commit a1dd3af into theochem:main Apr 19, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants