Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Pyiron job for Extra FIM simulations #8
base: main
Are you sure you want to change the base?
Pyiron job for Extra FIM simulations #8
Changes from 11 commits
d1f763d
6cdf9c7
a070d60
8d4eeb2
6a9fa17
6c78244
3dfb40e
1d7eeb1
77c1b7e
e842355
0a2c331
02f7127
e1bafd9
a508d4e
2abc181
0ff1814
3df3850
7eb8e13
6cb5d33
5f8bda8
21baa10
b7d4840
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding: this is the single-k computing job class, no? Should we reflect this in the class name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
essentially it is but, the user is not going to access this job. It is a reference job for the actual job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused about this if.
Is this function supposed to be called by the user before running, or as part of the running job (internal function)?
If this is not meant to be used by the user (who would rather set the input variables), the function should probably begin with an underscore to mark it as internal/private function. If this is to be run by user, where is the input dict set up to actually do something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was to give this as an user input. Default would be a boolean False, but if its True, then it calls the extrapolate potential function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a function, not a property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above. I suggest to move hd5 writing out of sum_single_k function and do it here explicitly (where we know that and where we want to write).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a direct copy of the function body further up. Is it possible to refactor such that we have a single implementation for both cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above: