-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add aggregate notebook that explains how to aggregate dynamically sized outputs #287
Conversation
1266698
to
ef939ca
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #287 +/- ##
==========================================
+ Coverage 75.75% 80.62% +4.87%
==========================================
Files 70 66 -4
Lines 4615 5147 +532
==========================================
+ Hits 3496 4150 +654
+ Misses 1119 997 -122
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Hi @agoscinski , thanks for the work! I requested a few changes.
f6aa32e
to
604a7c0
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.
Thanks for the update, almost ready to go. I added one last comment.
f03e123
to
317c704
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.
Because the graph builder example also uses the term dynamic input
I switched to usage dynamic input port
. Not sure if it should be dynamic input socket
.
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.
LGTM!
The notebook explains how to aggregate dynamically sized outputs and handle with nested datatypes as input.
Co-authored-by: Xing Wang <xingwang1991@gmail.com>
Co-authored-by: Xing Wang <xingwang1991@gmail.com>
Co-authored-by: Xing Wang <xingwang1991@gmail.com>
…ng difference between dynamic inputs and orm.Dict
Co-authored-by: Xing Wang <xingwang1991@gmail.com>
4cf04d8
to
e2574ea
Compare
9842670
to
e2574ea
Compare
Implements suggestions in issue #284
@superstar54 I have an issue withEDIT: I see that this is something related to my local setup and not a general bug.submit(wait=True)
the output is None (see first example where I kept submit).While I think the section about nested data types at the end is important, it does not really fit into the aggregate notebook. It is also more dealing with aiida-core issues than workgraph. I have no good idea at the moment. I would keep it there and see if we can move it somewhere more meaningful in the future.