Initial definitions for simulation workflow #121

JosePizarro3 · 2024-09-19T20:52:01Z

@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:

I moved SimulationWorkflow, SinglePoint, BeyondDFT and DFTPlusTB
I simplified quite a lot the logic in the original classes. I think this is a reflection of the inheritance hell one can enter, and I got really crazy trying to test stuff (you can see in Defining SimulationWorkflow, SinglePoint, BeyondDFT and DFTPlusTB workflows #120 my failed attempt on testing the original logics...). In any case, I think we can rework things along the development process when needed.
I did not move SerialWorkflow and ParallelWorkflow. Also related with the inheritance tight coupling. But we can recover them if needed later on.
I added an utils function to extract the main subsections of the archive.data.

When you want, you can review it. In the meantime, I will test the Wannier90 parser with this slightly new DFTPlusTB workflow section.

coveralls · 2024-09-19T20:53:52Z

Pull Request Test Coverage Report for Build 11269921175

Details

131 of 142 (92.25%) changed or added relevant lines in 5 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.7%) to 81.085%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/nomad_simulations/schema_packages/workflow/base_workflows.py	51	53	96.23%
src/nomad_simulations/schema_packages/workflow/single_point.py	22	24	91.67%
src/nomad_simulations/schema_packages/workflow/dft_plus_tb.py	45	48	93.75%
src/nomad_simulations/schema_packages/utils/utils.py	10	14	71.43%

Files with Coverage Reduction	New Missed Lines	%
src/nomad_simulations/schema_packages/utils/utils.py	1	78.57%

Totals
Change from base Build 11101838327:	0.7%
Covered Lines:	2182
Relevant Lines:	2691

💛 - Coveralls

Added DFTPlusTB and method sections for workflow

Added testing for BeyondDFTMethod

JFRudzinski

Maybe I will have more comments when I work on the MD part, but think the adjustments look fine to me at this point

JosePizarro3 · 2024-10-08T13:51:31Z

@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.

ladinesa · 2024-10-08T13:59:58Z

src/nomad_simulations/schema_packages/workflow/base_workflows.py

+        super().normalize(archive, logger)
+
+
+class BeyondDFTMethod(ArchiveSection):


No base class for method?

Well, this is just a place holder for referencing DFT, TB, DMFT, GW... sections in the archives of my workflows.

src/nomad_simulations/schema_packages/workflow/base_workflows.py

ladinesa · 2024-10-08T14:10:21Z

src/nomad_simulations/schema_packages/workflow/dft_plus_tb.py

+
+    @check_n_tasks(n_tasks=2)
+    def link_task_inputs_outputs(
+        self, tasks: list[TaskReference], logger: 'BoundLogger'


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

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

ladinesa · 2024-10-08T14:13:49Z

src/nomad_simulations/schema_packages/workflow/dft_plus_tb.py

+            tasks=self.tasks,
+            tasks_names=['DFT SinglePoint Task', 'TB SinglePoint Task'],
+        )
+        if method_refs is not None:


here, method_refs should only length 2 right and arranged as DFT, TN so maybe loop not needed

ladinesa · 2024-10-08T14:27:13Z

src/nomad_simulations/schema_packages/workflow/dft_plus_tb.py

+            return None
+
+        # Input of DFT Task is the ModelSystem
+        dft_task.inputs = [


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

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

ladinesa · 2024-10-08T14:28:09Z

looks good just minor clarifications needed

JosePizarro3 · 2024-10-10T10:16:57Z

@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 TB SinglePoint and DFTPlusTB. They share the same mainfile, do you think that's why the references are broken? Should I do something different when storing stuff in Link and TaskReference?

ladinesa · 2024-10-10T10:21:54Z

@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 TB SinglePoint and DFTPlusTB. They share the same mainfile, do you think that's why the references are broken? Should I do something different when storing stuff in Link and TaskReference?

but this is the idea of child archives and it should work

JosePizarro3 · 2024-10-10T11:27:30Z

@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 TB SinglePoint and DFTPlusTB. They share the same mainfile, do you think that's why the references are broken? Should I do something different when storing stuff in Link and TaskReference?

but this is the idea of child archives and it should work

Ok, then nvm :)

JosePizarro3 · 2024-12-16T11:48:08Z

@ladinesa @JFRudzinski I will be losing access to the mpcdf gitlab, and I doubt I want to keep it. Hence, this MR, https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/2143, will be stalled.

Feel free to either contact me for the Gitlab MR and ask for feedback or close it at will. But I wondered what are your plans for the base Workflow classes. If you don't want to change them, that's fine. So if they change, would you mind keeping me in the loop?

And if they don't change, I think you can safely close this issue/pr.

JosePizarro3 changed the title ~~100 different structure classes~~ Initial definitions for simulation workflow Sep 20, 2024

JosePizarro3 mentioned this pull request Sep 20, 2024

Defining SimulationWorkflow, SinglePoint, BeyondDFT and DFTPlusTB workflows #120

Closed

2 tasks

JosePizarro3 requested review from JFRudzinski and ladinesa September 20, 2024 13:03

JosePizarro3 added 23 commits October 2, 2024 10:08

Added SimulationWorkflow, SinglePoint

3f388cd

Added DFTPlusTB and method sections for workflow

Added testing for SimulationWorkflow

cd5cf6d

Move testing to subfolders to mimic structure in src

ce004d5

Rename testing file

c3f21ef

Added testing for BeyondDFTMethod

Changed name to BeyondDFT

d3042fe

Add testing for BeyondDFT workflow

03df03d

Fix resolve_inputs_outputs method

f520a9a

Add testing SinglePoint.generate_task method

61bef1b

Added testing for SinglePoint methods

64d85f6

Fix types

6a03e4c

Added check_n_tasks decorator

c34705f

Added testing for link_tasks

e313bb1

Added todo for testing overwrite_fermi_level once this is under control

8e80dde

Initial idea (equivalent to the workflow-task schema

8eff916

Added utils extract_simulation_subsections

4213e99

Fix base_workflows and testing

c64c894

Change name to extract_all_simulation_subsections

cc7a8ee

Fix single_point and testing

d39c525

Fix imports

304d344

Add resolve_method_refs method to BeyondDFT

95fdf8d

Fix dft_plus_tb and testing

a3b95bf

Added more testing and comments

d2d57ff

Rebase and delete copyright notice

6a7668b

JosePizarro3 force-pushed the 100-different-structure-classes branch from 64ae0ef to 6a7668b Compare October 2, 2024 08:11

Deleting copyright text

14b982b

JFRudzinski approved these changes Oct 4, 2024

View reviewed changes

ladinesa reviewed Oct 8, 2024

View reviewed changes

src/nomad_simulations/schema_packages/workflow/base_workflows.py Show resolved Hide resolved

ladinesa reviewed Oct 8, 2024

View reviewed changes

src/nomad_simulations/schema_packages/workflow/base_workflows.py Show resolved Hide resolved

ladinesa reviewed Oct 8, 2024

View reviewed changes

Added comments

ad055ee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial definitions for simulation workflow #121

Initial definitions for simulation workflow #121

JosePizarro3 commented Sep 19, 2024 •

edited

Loading

coveralls commented Sep 19, 2024 •

edited

Loading

JFRudzinski left a comment

JosePizarro3 commented Oct 8, 2024

ladinesa Oct 8, 2024

JosePizarro3 Oct 9, 2024

ladinesa Oct 8, 2024 •

edited

Loading

JosePizarro3 Oct 10, 2024

ladinesa Oct 8, 2024

ladinesa Oct 8, 2024

JosePizarro3 Oct 10, 2024

ladinesa commented Oct 8, 2024

JosePizarro3 commented Oct 10, 2024

ladinesa commented Oct 10, 2024

JosePizarro3 commented Oct 10, 2024

JosePizarro3 commented Dec 16, 2024

		super().normalize(archive, logger)


		class BeyondDFTMethod(ArchiveSection):

Initial definitions for simulation workflow #121

Are you sure you want to change the base?

Initial definitions for simulation workflow #121

Conversation

JosePizarro3 commented Sep 19, 2024 • edited Loading

coveralls commented Sep 19, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11269921175

Details

💛 - Coveralls

JFRudzinski left a comment

Choose a reason for hiding this comment

JosePizarro3 commented Oct 8, 2024

ladinesa Oct 8, 2024

Choose a reason for hiding this comment

JosePizarro3 Oct 9, 2024

Choose a reason for hiding this comment

ladinesa Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Oct 10, 2024

Choose a reason for hiding this comment

ladinesa Oct 8, 2024

Choose a reason for hiding this comment

ladinesa Oct 8, 2024

Choose a reason for hiding this comment

JosePizarro3 Oct 10, 2024

Choose a reason for hiding this comment

ladinesa commented Oct 8, 2024

JosePizarro3 commented Oct 10, 2024

ladinesa commented Oct 10, 2024

JosePizarro3 commented Oct 10, 2024

JosePizarro3 commented Dec 16, 2024

JosePizarro3 commented Sep 19, 2024 •

edited

Loading

coveralls commented Sep 19, 2024 •

edited

Loading

ladinesa Oct 8, 2024 •

edited

Loading