-
Notifications
You must be signed in to change notification settings - Fork 1
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
Initial definitions for simulation workflow #121
base: develop
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 11269921175Details
💛 - Coveralls |
Added DFTPlusTB and method sections for workflow
Added testing for BeyondDFTMethod
64ae0ef
to
6a7668b
Compare
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.
Maybe I will have more comments when I work on the MD part, but think the adjustments look fine to me at this point
@ladinesa I might need to merge this soon so the TMM can test stuff. Can you review it? I know the refs are bugged, but that's another issue, not related with the schema. |
super().normalize(archive, logger) | ||
|
||
|
||
class BeyondDFTMethod(ArchiveSection): |
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.
No base class for method?
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.
Well, this is just a place holder for referencing DFT, TB, DMFT, GW... sections in the archives of my workflows.
|
||
@check_n_tasks(n_tasks=2) | ||
def link_task_inputs_outputs( | ||
self, tasks: list[TaskReference], logger: 'BoundLogger' |
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 thought you already figured out how to handle loggers. since you are using decorators, maybe create a decorator for error handling as nathan suggested
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.
Well, the loggers issue is after this one. I can change to the class __init__
; I find it more convenient to use self.logger
for all methods
tasks=self.tasks, | ||
tasks_names=['DFT SinglePoint Task', 'TB SinglePoint Task'], | ||
) | ||
if method_refs is not None: |
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.
here, method_refs should only length 2 right and arranged as DFT, TN so maybe loop not needed
return None | ||
|
||
# Input of DFT Task is the ModelSystem | ||
dft_task.inputs = [ |
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.
not sure if i understood this right but it should be the other way around right? i.e you assign self.in(out)puts from dft and tb task inputs and outputs
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.
After our discussion, this is what was confusing me with the need of defining inputs/outputs in a TaskReference.
I think I can improve this in a new schema as specified in https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/2143 with some minor tweeks
looks good just minor clarifications needed |
@ladinesa thanks! I have another branch I will eventually merge here, I took into account your comments. I have a question tho: right now, the Wannier90 parser splits the entries for |
but this is the idea of child archives and it should work |
Ok, then nvm :) |
@ladinesa @JFRudzinski this is my attempt to move some classes and simplify stuff from the simulation workflow package. Sorry for the changes, I also moved testing files to subfolders (so please, disregard the changes for the subfolders
tests/properties
).Some list of changes:
SimulationWorkflow
,SinglePoint
,BeyondDFT
andDFTPlusTB
SerialWorkflow
andParallelWorkflow
. Also related with the inheritance tight coupling. But we can recover them if needed later on.When you want, you can review it. In the meantime, I will test the Wannier90 parser with this slightly new
DFTPlusTB
workflow section.