-
Notifications
You must be signed in to change notification settings - Fork 187
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
✨ NEW: _repr_pretty_ representation for ProcessBuilder
#4970
Conversation
This is one I've been meaning to do for a while now, but I never knew about @sphuber this is adapted from a method I stole from the Some usage examples:
builder = code.get_builder()
builder.structure = structure
builder.parameters = orm.Dict(dict={
'CONTROL': {
'calculation': 'scf', # self-consistent field
},
'SYSTEM': {
'ecutwfc': 30., # wave function cutoff in Ry
'ecutrho': 240., # density cutoff in Ry
}
})
builder Gives
code = orm.load_code(1)
structure = orm.load_node(3)
builder = PwBaseWorkChain.get_builder_from_protocol(
code=code,
structure=structure,
)
builder Gives:
|
@chrisjsewell should I add something to ignore these
|
Just for reference, this is what you would get for the
(quoted here to wrap the super long line) |
So that's what it would look like in ipython and jupyter notebooks? I love it! |
Codecov Report
@@ Coverage Diff @@
## develop #4970 +/- ##
===========================================
+ Coverage 77.21% 82.13% +4.93%
===========================================
Files 533 533
Lines 38470 38485 +15
===========================================
+ Hits 29700 31607 +1907
+ Misses 8770 6878 -1892
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Unfortunately, not in IPython. 😞 The console doesn't support HTML, notebooks do. See here. |
|
👀 Indeed! Let me get right on that. 😁 |
@louisponet there you go! I dumped the |
Note: the |
ProcessBuilder
: Add HTML representationProcessBuilder
: Add pretty representation 😍
changing this to a draft while the tests are failing @mbercx. Get the tests passed then we can finalise this I guess 😄 |
f93dffc
to
cb0a906
Compare
Hmm, so I can't seem to fix the pre-commit because if I actually try to ignore these issues: aiida/engine/processes/builder.py:96: error: Incompatible types in assignment (expression has type "None", variable has type "Union[Port, PortNamespace]") [assignment]
aiida/engine/processes/builder.py:98: error: Item "Port" of "Union[Port, PortNamespace]" has no attribute "serialize" [union-attr]
aiida/engine/processes/builder.py:98: error: Item "PortNamespace" of "Union[Port, PortNamespace]" has no attribute "serialize" [union-attr]
Found 3 errors in 1 file (checked 74 source files) By adding aiida/engine/processes/builder.py:96: error: unused 'type: ignore' comment
aiida/engine/processes/builder.py:98: error: unused 'type: ignore' comment
Found 2 errors in 1 file (checked 1 source file) Don't know how to fix this due to my |
Might be related to this issue: #5026 I have also suffered from this a few times and have no idea where it comes from nor how to fix it. |
How do I never seem to have these issues 😆 |
sigh, alright! I'll finally start looking into using |
c32df03
to
11aeaab
Compare
(oops, slippery fingers! 😅 ) |
Actually, maybe it makes a lot more sense to output this in a YAML format... It's meant to be human readable after all. For the See pretty representation of builderProcess class: PwBandsWorkChain
Inputs:
bands:
metadata: {}
pw:
code: Plane wave Quantum ESPRESSO v6.6 - local compilation
metadata:
options:
max_wallclock_seconds: 43200
resources:
num_machines: 1
stash: {}
withmpi: true
parameters:
CONTROL:
calculation: scf
etot_conv_thr: 2.0e-05
forc_conv_thr: 0.0001
tprnfor: true
tstress: true
ELECTRONS:
conv_thr: 4.0e-10
electron_maxstep: 80
mixing_beta: 0.4
SYSTEM:
degauss: 0.01
ecutrho: 240.0
ecutwfc: 30.0
nosym: false
occupations: smearing
smearing: cold
pseudos:
Si: ''
bands_kpoints_distance: 0.025
clean_workdir: true
metadata: {}
nbands_factor: 3.0
relax:
base:
kpoints_distance: 0.15
kpoints_force_parity: false
metadata: {}
pw:
code: Plane wave Quantum ESPRESSO v6.6 - local compilation
metadata:
options:
max_wallclock_seconds: 43200
resources:
num_machines: 1
stash: {}
withmpi: true
parameters:
CELL:
cell_dofree: all
press_conv_thr: 0.5
CONTROL:
calculation: vc-relax
etot_conv_thr: 2.0e-05
forc_conv_thr: 0.0001
tprnfor: true
tstress: true
ELECTRONS:
conv_thr: 4.0e-10
electron_maxstep: 80
mixing_beta: 0.4
SYSTEM:
degauss: 0.01
ecutrho: 240.0
ecutwfc: 30.0
nosym: false
occupations: smearing
smearing: cold
pseudos:
Si: ''
base_final_scf:
metadata: {}
pw:
metadata:
options:
stash: {}
pseudos: {}
max_meta_convergence_iterations: 5
meta_convergence: true
metadata: {}
volume_convergence: 0.02
scf:
kpoints_distance: 0.15
kpoints_force_parity: false
metadata: {}
pw:
code: Plane wave Quantum ESPRESSO v6.6 - local compilation
metadata:
options:
max_wallclock_seconds: 43200
resources:
num_machines: 1
stash: {}
withmpi: true
parameters:
CONTROL:
calculation: scf
etot_conv_thr: 2.0e-05
forc_conv_thr: 0.0001
tprnfor: true
tstress: true
ELECTRONS:
conv_thr: 4.0e-10
electron_maxstep: 80
mixing_beta: 0.4
SYSTEM:
degauss: 0.01
ecutrho: 240.0
ecutwfc: 30.0
nosym: false
occupations: smearing
smearing: cold
pseudos:
Si: ''
structure: Si |
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 @mbercx just two minor comments remaining. And then if you are still up for it, I would take you up on your suggestion to use YAML instead of JSON.
The representation of the `ProcessBuilder` (i.e. `__repr__`) is difficult to read since it just prints the heavily nested mapping on one line. Here we add a `_repr_html_` method to improve the readability when using the `ProcessBuilder` in e.g. Jupyter notebooks.
for more information, see https://pre-commit.ci
e94c13c
to
ff48c17
Compare
"""Pretty representation for in the IPython console and notebooks.""" | ||
return p.text( | ||
f'Process class: {self._process_class.__name__}\n' | ||
f'Inputs:\n{yaml.safe_dump(json.JSONDecoder().decode(PrettyEncoder().encode(self)))}' |
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.
Doing a full encode/decode cycle in JSON before encoding in YAML is a bit inefficient. Then again, writing serializers in YAML is a bit of a mess (like in aiida.orm.utils.serialize
, and unfortunately we cannot even reuse that) and this should not be horribly costly, so let's leave this as is.
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.
Yeah, I know it's a bit of a messy workaround, but didn't know how to write the serializer in YAML to be honest. I timed the line with %%timeit
and it only took a couple dozen nanoseconds, so should be ok. ^^
Thanks a lot @mbercx ! |
The representation of the
ProcessBuilder
(i.e.__repr__
) isdifficult to read since it just prints the heavily nested mapping on one
line and doesn't show the contents of e.g.
Dict
nodes.Here we add a
_repr_pretty_
method to improve the readabilitywhen using the
ProcessBuilder
in Jupyter notebooks and IPython.