-
Notifications
You must be signed in to change notification settings - Fork 202
Remove fields from PICMI ParticleDiagnostic #3207
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
base: development
Are you sure you want to change the base?
Conversation
Python/pywarpx/picmi.py
Outdated
@@ -1335,6 +1335,7 @@ def initialize_inputs(self): | |||
pywarpx.diagnostics._diagnostics_dict[self.name] = self.diagnostic | |||
|
|||
self.diagnostic.diag_type = 'Full' | |||
self.diagnostic.fields_to_plot = '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.
Thanks for posting this PR. This new line should go after line 1334 that creates a new Diagnostic
instance. If a previously created Diagnostic
instance is fetched (in line 1332) it may have be made by a FieldDiagnostic
and so already have fields_to_plot
defined.
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! Looks good!
@dpgrote It seems that the CI tests fail with the following error:
This is a Python test, with a single diagnostic - which is a |
I think that Yt is looking for the Header file, but that file wasn't written out since there are no fields (the AMReX routine that would write out that file was not called). This will be a problem for the no fields option since it means that the plot files cannot be processed with Yt. This is something that was not noticed in PR #2419. For the failing test, a simple fix is to not have Yt open the plot file. I'll look at this a bit and see if I can find a way around this issue. |
This should now work as PR #3219 has been merged. Note that the CI benchmark file will need to be updated to remove the references to the fields. |
ac4d415
to
3a2686d
Compare
@@ -1891,6 +1891,7 @@ def add_diagnostic(self): | |||
self.diagnostic = pywarpx.Diagnostics.Diagnostic( | |||
self.name, _species_dict={} | |||
) | |||
self.diagnostic.fields_to_plot = '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.
I saw that this PR was revived. I don't think this is the right place for this line since WarpXDiagnosticBase
is used by all of the diagnostic classes. This setting would mess up Checkpoint
since fields_to_plot
shouldn't be set for it. Also, fields_to_plot
is not used by ReducedDiagnostic
. The better place for this statement is in ParticleDiagnostic
after the call to self.add_diagnostic()
, adding it this way
if 'fields_to_plot' not in self.diagnostic.argvattrs:
self.diagnostic.fields_to_plot = 'none'
ParticleDiagnostic
LabFrameParticleDiagnostic
LabFrameFieldDiagnostic