-
Notifications
You must be signed in to change notification settings - Fork 12
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
Packaging of Graphium 3.0 #531
base: graphium_3.0
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a few minor requests.
Also, I feel that the Graphium-cpp main page should not be empty. Perhaps a text stating:
Welcome to the documentation for the C++ API of the graphium library. For the python API, go to this linkhttps://graphium-docs.datamol.io/stable/). The C++ API is used to significantly accelerate the computation of positional and structural encodings, but also the caching of the supervised labels into very light and optimized files. This significantly accelerates the dataloading and removes the need for long pre-processing of large datasets.
README.md
Outdated
### conda-forge | ||
conda-forge is the recommended method for installing Graphium. To install Graphium via conda-forge, run the following command: | ||
``` | ||
conda install graphium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conda install graphium | |
mamba install graphium -c conda-forge |
env.yml
Outdated
- tqdm | ||
- platformdirs | ||
|
||
# scientific | ||
- numpy == 1.26.4 | ||
- numpy=1.25.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- numpy=1.25.0 | |
- numpy <= 1.25.0 |
Too strict
env.yml
Outdated
- mup | ||
- pytorch_sparse >=0.6 | ||
- pytorch_cluster >=1.5 | ||
- pytorch_scatter >=2.0 | ||
|
||
# chemistry | ||
- rdkit == 2024.03.4 | ||
- rdkit=2024.03.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too strict. Can you do <=
or >=
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll run a test build with <=
and report back.
19e4dca
to
55b4076
Compare
533c770
to
90dc934
Compare
474c802
to
3a10b45
Compare
@DomInvivo applied your suggestions and got the tests running for all operating systems we'll support. It's ready for another quick round of review. Please remember to build the docs and checkout the changes I made to the C++ API docs. |
Changelogs
setup.py
to buildgraphium_cpp
as part of thegraphium
build process.setup.py
to allow propergraphium_cpp
compilation.doxygen
forgraphium_cpp
doc building. Configured to integrate into existing docs deployment.(@DomInvivo I would pull this branch and runmkdocs serve
to ensure it all looks good to you. There is a nav menu button named "Graphium C++").env.yml
file updated with dependencies/versions that are compatible withgraphium
andgraphium_cpp
.README.md
to reflect how to install3.0.0
. A note was added about PyPi version limitation.Checklist:
feature
,fix
ortest
(or ask a maintainer to do it for you).discussion related to that PR
Important Notes
graphium
documentation given that he has the most context about what features are available and not in the package. We looked today and some work was needed.graphium/trainer/metrics.py
. Is this correct?3.9
,3.10
and3.11
right now. Becausepyg
is pinning us tonumpy
version1.25.0
, we cannot use3.12
(not compatible with that version ofnumpy
). Oncepyg
makes a new release, I will update our conda-forge recipe to build for3.12
.tests/test_datamodule.py
. I had to update two assert statements to get passing tests. Was this update correct or did I just make a failing test pass?doxygen
.