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

Fix sqsgenerator version bump #569

Merged
merged 13 commits into from Apr 11, 2022

Conversation

dgehringer
Copy link
Contributor

Fixes SQSJob to work with both versions v0.0.5 and v0.1.

  • left HDF format and input parameters untouched -> backwards compatibilty
  • deprecation warning whe importing v0.0.5
  • CI notebook sqs.ipynb runs with both versions ( v0.0.5 and v0.1)
  • improved input validation -> warnings if composition is changes
  • new features of v0.1 are not included -> backwards compatibilty

Fixes #564

@dgehringer dgehringer changed the title Fix sqsgenerator version bump #564 Fix sqsgenerator version bump Mar 21, 2022
@coveralls
Copy link

coveralls commented Mar 21, 2022

Pull Request Test Coverage Report for Build 2017139991

  • 19 of 76 (25.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 70.325%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/atomistics/job/sqs.py 19 76 25.0%
Files with Coverage Reduction New Missed Lines %
pyiron_atomistics/atomistics/job/sqs.py 1 39.53%
Totals Coverage Status
Change from base Build 2003890928: -0.2%
Covered Lines: 11875
Relevant Lines: 16886

💛 - Coveralls

@jan-janssen jan-janssen added the format_black reformat the code using the black standard label Mar 21, 2022
@jan-janssen jan-janssen added format_black reformat the code using the black standard and removed format_black reformat the code using the black standard labels Mar 21, 2022
@jan-janssen
Copy link
Member

The coverage fails to push the test statistics - but I guess that is a general issue with forks

@jan-janssen
Copy link
Member

The backwards compatibility to 0.0.5 seems to be broken:

File ~/work/pyiron_atomistics/pyiron_atomistics/pyiron_atomistics/atomistics/job/sqs.py:354, in SQSJob.run_static(self)
[340](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:340)
    353 def run_static(self):
[341](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:341)
--> 354     self._lst_of_struct, decmp, iterations, cycle_time = get_sqs_structures(
[342](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:342)
    355         structure=self.structure,
[343](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:343)
    356         mole_fractions={k: v for k, v in self.input.mole_fractions.items()},
[344](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:344)
    357         weights=self.input.weights,
[345](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:345)
    358         objective=self.input.objective,
[346](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:346)
    359         iterations=self.input.iterations,
[347](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:347)
    360         output_structures=self.input.n_output_structures,
[348](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:348)
    361         num_threads=self.server.cores,
[349](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:349)
    362     )
[350](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:350)
    363     for i, structure in enumerate(self._lst_of_struct):
[351](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:351)
    364         with self.project_hdf5.open("output/structures/structure_" + str(i)) as h5:
[352](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:352)

[353](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:353)
File ~/work/pyiron_atomistics/pyiron_atomistics/pyiron_atomistics/atomistics/job/sqs.py:138, in get_sqs_structures(structure, mole_fractions, weights, objective, iterations, output_structures, num_threads)
[354](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:354)
    136 # preprocess mole-fractions, to print warnings is necessary
[355](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:355)
    137 num_atoms = len(structure)
[356](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:356)
--> 138 composition = mole_fractions_to_composition(mole_fractions, num_atoms)
[357](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:357)
    139 mole_fractions = map_dict(lambda n: n/num_atoms, composition)
[358](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:358)
    141 iterator = ParallelSqsIterator(
[359](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:359)
    142     structure, mole_fractions, weights, num_threads=num_threads
[360](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:360)
    143 )
[361](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:361)

[362](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:362)
File ~/work/pyiron_atomistics/pyiron_atomistics/pyiron_atomistics/atomistics/job/sqs.py:105, in mole_fractions_to_composition(mole_fractions, num_atoms)
[363](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:363)
     99     warnings.warn(
[364](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:364)
    100         'It is not possible to distribute the species properly. Therefore one "{}" atom was removed. '
[365](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:365)
    101         'This changes the input mole-fraction specification. '
[366](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:366)
    102         '"{}" -> "{}"'.format(removed_species, mole_fractions, map_dict(lambda n: n/num_atoms, composition)))
[367](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:367)
    103 else:
[368](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:368)
    104     # something else is wrong with the mole-fractions input
[369](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:369)
--> 105     raise ValueError('Cannot interpret mole-fraction dict {}'.format(mole_fractions))
[370](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:370)
    107 return composition
[371](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:371)

[372](https://github.com/pyiron/pyiron_atomistics/runs/5625294004?check_suite_focus=true#step:6:372)
ValueError: Cannot interpret mole-fraction dict {'Ni': 0.8, 'Cr': 0.2}

@jan-janssen
Copy link
Member

Also with the latest version the same error message appears. Can this be related to the input parameters being a DataContainer rather than a python dictionary? Maybe @pmrv can help.

@dgehringer
Copy link
Contributor Author

The backwards compatibility to 0.0.5 seems to be broken:

That should be fixed. It was caused by a wrong condition

Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black reformat the code using the black standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI Notebooks broken
4 participants