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

DFT initial pr - Adding utilities #3190

Merged
merged 11 commits into from
Feb 6, 2023
Merged

DFT initial pr - Adding utilities #3190

merged 11 commits into from
Feb 6, 2023

Conversation

advikavs
Copy link
Contributor

Description

Adding utilities for XCNN model.

retain_graph=True)
return f

def hashstr(s: str) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

This should have a docstring

from dqc.qccalc.base_qccalc import BaseQCCalc
import hashlibs

class KSCalc(object):
Copy link
Member

Choose a reason for hiding this comment

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

We should add some basic unit tests for this object

@advikavs advikavs changed the title DFT initial pr DFT initial pr - Adding utilities Jan 23, 2023
import torch
from dqc.utils.datastruct import SpinParam
from dqc.qccalc.base_qccalc import BaseQCCalc
except ImportError:
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be ModuleNotFoundError

def energy(self) -> torch.Tensor:
"""
Returns
_______
Copy link
Contributor

Choose a reason for hiding this comment

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

hyphens (-) over underscores (_)

from deepchem.utils.dftutils import KSCalc, hashstr
import torch


Copy link
Contributor

Choose a reason for hiding this comment

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

@pytest.mark.torch because dqc requires pytorch to be installed

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are running dqc tests in a separate workflow, we should have it marked as @pytest.mark.dqc and then invoke the test in dqc workflow.

@arunppsg
Copy link
Contributor

We should add in deepchem/utils/__init__.py the imports from dftutils.py under a try-except block.

@arunppsg arunppsg merged commit b03e2d3 into deepchem:master Feb 6, 2023
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.

None yet

3 participants