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

Move FAQ from Toolkit #63

Merged
merged 10 commits into from
Sep 10, 2024
Merged

Move FAQ from Toolkit #63

merged 10 commits into from
Sep 10, 2024

Conversation

Yoshanuikabundi
Copy link
Contributor

@Yoshanuikabundi Yoshanuikabundi commented Sep 10, 2024

This PR re-organizes and replaces the Toolkit FAQ to be part of the top level docs.

Rendered: https://openff-docs--63.org.readthedocs.build/en/63/faq.html

Todo:

Copy link

This repository caches data generated from notebooks stored in other repositories in the _cookbook_data_* branches. Regenerating this cache takes about half an hour, so it is not done automatically in PRs. The current value of the cache from the main branch has been copied over to this PR's cache.

To regenerate the cache for this PR, create a comment on this PR consisting only of the string:

/regenerate-cache

@Yoshanuikabundi
Copy link
Contributor Author

@j-wags I've made some edits to what Matt brought up in openforcefield/openff-toolkit#1848, so I'm going to leave this unmerged for tonight in case he has corrections - I should be able to merge it in my AM before your talk.

@mattwthompson - sorry for dropping off the face of the planet and then asking for a quick response :/ I'm happy to merge this as is and you can get around to checking it at your leisure - I'm confident its correct, just not sure it addresses exactly the points you wanted to address.

@Yoshanuikabundi
Copy link
Contributor Author

Do we have a way to remove parameters from a force field? We've got del in the FAQ:

del force_field['Constraints']["[#1:1]-[*:2]"]

But it doesn't work. Does anyone know if this is a change or did this just never work?

@mattwthompson
Copy link
Member

I can get below to work with some effort, which I can't say would make me super happy as a user:

In [13]: sage = ForceField("openff-2.0.0.offxml")

In [14]: sage['Constraints']["[#1:1]-[*:2]"]
Out[14]: <ConstraintType with smirks: [#1:1]-[*:2]  id: c1  >

In [15]: sage["Constraints"].parameters.index(sage['Constraints']["[#1:1]-[*:2]"])
Out[15]: 0  # not relevant, just checking

In [16]: del sage['Constraints'].parameters["[#1:1]-[*:2]"]

In [17]: sage['Constraints']["[#1:1]-[*:2]"]
---------------------------------------------------------------------------
ParameterLookupError                      Traceback (most recent call last)
Cell In[17], line 1
----> 1 sage['Constraints']["[#1:1]-[*:2]"]

File ~/micromamba/envs/evaluator-test-env/lib/python3.11/site-packages/openff/toolkit/typing/engines/smirnoff/parameters.py:2439, in ParameterHandler.__getitem__(self, val)
   2434 def __getitem__(self, val):
   2435     """
   2436     Syntax sugar for looking up a ParameterType in a ParameterHandler
   2437     based on its SMIRKS.
   2438     """
-> 2439     return self.parameters[val]

File ~/micromamba/envs/evaluator-test-env/lib/python3.11/site-packages/openff/toolkit/typing/engines/smirnoff/parameters.py:1574, in ParameterList.__getitem__(self, item)
   1572     indexable_item: Union[int, slice] = item  # type: ignore[assignment]
   1573 elif isinstance(item, str):
-> 1574     indexable_item = self.index(item)
   1575 elif isinstance(item, ParameterType) or issubclass(type(item), ParameterType):
   1576     raise ParameterLookupError("Lookup by instance is not supported")

File ~/micromamba/envs/evaluator-test-env/lib/python3.11/site-packages/openff/toolkit/typing/engines/smirnoff/parameters.py:1529, in ParameterList.index(self, item, start, stop)
   1527     if parameter.smirks == item:
   1528         return self.index(parameter)
-> 1529 raise ParameterLookupError(f"SMIRKS {item} not found in ParameterList")

ParameterLookupError: SMIRKS [#1:1]-[*:2] not found in ParameterList

I think openforcefield/openff-toolkit#1680 is related if we want to directly add this functionality to pass through ParameterHandlers - notably class ParameterList(list):, but other handy features allow users to bypass explicitly calling out .parameters

Copy link
Member

@mattwthompson mattwthompson left a comment

Choose a reason for hiding this comment

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

LGTM; I sought out only the section headers that seemed relevant to the questions I asked back in march

source/index.md Show resolved Hide resolved
source/faq.md Outdated Show resolved Hide resolved
source/faq.md Outdated Show resolved Hide resolved
@Yoshanuikabundi Yoshanuikabundi merged commit b8bd433 into main Sep 10, 2024
2 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