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

Pad Representation with zeros for non-present species #350

Open
rosecers opened this issue May 6, 2021 · 5 comments · May be fixed by #353
Open

Pad Representation with zeros for non-present species #350

rosecers opened this issue May 6, 2021 · 5 comments · May be fixed by #353
Assignees
Labels
documentation Needs addtional documentation enhancement New feature or request

Comments

@rosecers
Copy link
Contributor

rosecers commented May 6, 2021

I have this problem quite often, where I'm computing a subset of SOAP vectors for who species ABCD are present, then another subset where only ABD are present. I know that I can use the species pair dictionary to collect SOAPs per species pair, but it would be far, far more convenient to do something like this:

hyperparameters = {
    "soap_type": "PowerSpectrum",
    ...

    # I know this keyword exists, but seems to only affect `expansion_by_species_method`
    "global_species": [A, B, C, D, ...] 
}
soap = SOAP(**hyperparameters)

featuresABCD = soap.transform(framesABCD).get_features(soap)
featuresABD = soap.transform(framesABD).get_features(soap)

where featuresABCD and featuresABD have the same indexing and shape.

@rosecers rosecers added the enhancement New feature or request label May 6, 2021
@max-veit
Copy link
Contributor

max-veit commented May 6, 2021

For global_species to work, I think you also have to set expansion_by_species_method="user defined" in the hypers. Is that what you meant by the comment in your example?

@max-veit max-veit added the documentation Needs addtional documentation label May 6, 2021
@ceriottm
Copy link
Contributor

ceriottm commented May 6, 2021 via email

@rosecers
Copy link
Contributor Author

rosecers commented May 7, 2021 via email

@max-veit max-veit self-assigned this May 7, 2021
max-veit added a commit that referenced this issue May 7, 2021
(it was ignored before if 'expansion_by_species_method' was not set
appropriately; this is now done automatically)

Fix #350
@max-veit
Copy link
Contributor

max-veit commented May 7, 2021

Ok, this is now implemented in bugfix/soap_global_species. Let me know if you want to add tests as well; I think that could be a good idea.

@rosecers
Copy link
Contributor Author

Looking at the code, I have a few thoughts (first of which is turning the branch into a draft PR).

  • Is there a need to have the two separate input parameters (global_species and expansion_by_species_method) if inputting one overrides the other? Why not have an option to expansion_by_species_method that anticipates a list of global species?
  • If kept two parameters, I would suggest a warning in lines 219-220

@max-veit max-veit linked a pull request May 10, 2021 that will close this issue
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Needs addtional documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants