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

Use input structure for get_number_of_species #1350

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

Use input structure for get_number_of_species #1350

wants to merge 1 commit into from

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Mar 1, 2024

This fixes an issue where tables are getting slowed down a lot for jobs run before #1341 was merged. For those jobs output/structure/species is missing and accidentally trigger decompression of the jobs.

This fixes an issue where tables are getting slowed down a lot for jobs run before
#1341 was merged.  For those jobs output/structure/species is missing and accidentally
trigger decompression of the jobs.
@pmrv pmrv added the bug Something isn't working label Mar 1, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8117105299

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.174%

Totals Coverage Status
Change from base Build 8108226849: 0.0%
Covered Lines: 14209
Relevant Lines: 15250

💛 - Coveralls

@jan-janssen
Copy link
Member

Initially I thought this breaks the compatibility with the SQSJob but it stores its structures in output/structures so it is not effected. Still this made me wonder if it would be better to search for a structure in multiple places, start with output/structure then maybe output/structures and finally input/structure. Another case where this breaks the functionality is a LAMMPS calculation with variational grand canonical sampling.

@jan-janssen jan-janssen marked this pull request as draft March 4, 2024 11:47
@pmrv
Copy link
Contributor Author

pmrv commented Mar 22, 2024

Initially I thought this breaks the compatibility with the SQSJob but it stores its structures in output/structures so it is not effected. Still this made me wonder if it would be better to search for a structure in multiple places, start with output/structure then maybe output/structures and finally input/structure.

I can do that. Personally I like input/, because that's somewhat consistent with the other predefined functions.

Another case where this breaks the functionality is a LAMMPS calculation with variational grand canonical sampling.

In a round about way actually no, but only because in case of SGC MC a lot of our code breaks when not all elements are present in the initial structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants