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

Handle passing/initialization of reference to matchms environment #13

Open
hechth opened this issue Mar 10, 2022 · 3 comments
Open

Handle passing/initialization of reference to matchms environment #13

hechth opened this issue Mar 10, 2022 · 3 comments

Comments

@hechth
Copy link
Collaborator

hechth commented Mar 10, 2022

At first I also wanted to drop it - but then I realized:
this allows also to use import("matchms", convert = FALSE) to disable automatic conversion of python to R and vice versa, maybe not wrong to disable to speed up things?
Also, in a loop, you can define this reference once, and then in the loop re-use it. Speeds up things.
I would thus keep it (it's anyway for the advanced user).

I think having a function that populates a global variable might be the best way?

I don't think that passing the reference to the imported matchms is a good thing - this implies you could use different versions of matchms in a single session etc., which is not very clean.

But we can figure out how to do this properly in another issue.

Originally posted by @hechth in #10 (comment)

@jorainer
Copy link
Member

What I've seen so far in other packages is that they call the import within each function. It's just not very efficient if you repeatedly do that in a loop. At present the conversion is done on each spectrum individually in such a loop (which is also not that efficient).

I totally agree that using different versions of matchms would be problematic - but here I trust the user - these function should only be for the advanced user, and they, hopefully, know what they are doing.

I am not in favour of a global variable, because that might prevent us from using parallel processing (well, question is also if we would really want to do that...).

@hechth
Copy link
Collaborator Author

hechth commented Mar 10, 2022

Well we can export the variable to the compute cluster, right? Or is that one of the things the biocparallel package can't do? I have the vague memory that there was something like that.

@jorainer
Copy link
Member

it depends on the cluster setup - if it has shared memory or not (on Windows, i.e. with socket-based clustering, there is no shared memory).

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

No branches or pull requests

2 participants