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

Remove unserializable default params for pattern and layered fingerprints #112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alkorolyov-selvita
Copy link
Contributor

@alkorolyov-selvita alkorolyov-selvita commented Nov 20, 2024

Removed unhashable list [] params due to failure to use pattern and layered fingerprint calculators in dm.parallelized() and in FPVecTransformer(n_jobs=-1).

Changelogs

Remove list [] params from FP_DEF_PARAMS for layerd and pattern fingerprints.

Removed unhashable list params due to failure to run `pattern` and `layered` fingerprint calculators when using dm.parallelized().
@zhu0619
Copy link
Contributor

zhu0619 commented Nov 25, 2024

@alkorolyov-selvita
I can't reproduce the issue.
This is the code that works for me.

import datamol as dm
from molfeat.trans.fp import FPVecTransformer
smiles = dm.freesolv().iloc[:100].smiles
transformer = FPVecTransformer(kind='pattern', dtype=float)

dm.parallelized(fn=transformer, inputs_list=smiles, n_jobs=-1)

Could you post the code which causes the issue for you here ?

@alkorolyov-selvita
Copy link
Contributor Author

alkorolyov-selvita commented Nov 26, 2024

@zhu0619
Of course, I create a dedicated issue with detailed logs and examples #113
quick example of failed code on Windows (it seems to work fine on Ubuntu):

import datamol as dm
from molfeat.trans import FPVecTransformer, FeatConcat

trans = FPVecTransformer('pattern', n_jobs=-1)

df = dm.data.freesolv().iloc[:100]

trans(df.smiles).shape

@zhu0619
Copy link
Contributor

zhu0619 commented Nov 27, 2024

I am able to reproduce the error now.
Removing the default parameters might solve the issue temporarily.
The serialization issue still exists if customized FPCalculator parameters are provided.

I would say to update the __setstate__ function to deal with any list arguments.

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