Skip to content
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

Improvements: Post-Processing revamp #18

Merged
merged 43 commits into from
Sep 7, 2024

Conversation

ShaunFell
Copy link
Owner

This PR will add more documentation for the post processing routines

@ShaunFell ShaunFell linked an issue Jul 19, 2024 that may be closed by this pull request
10 tasks
@ShaunFell
Copy link
Owner Author

This PR fixes #13

@ShaunFell ShaunFell removed a link to an issue Jul 19, 2024
10 tasks
@ShaunFell ShaunFell changed the title added documentation for 2D plotter added documentation for post-processing routines Jul 19, 2024
@ShaunFell ShaunFell mentioned this pull request Jul 26, 2024
10 tasks
@ShaunFell ShaunFell changed the title added documentation for post-processing routines Post-Processing Improvements Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably remove the .pyc files and add .pyc to your `.gitignore

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats strange, because the .gitignore already contains those. I manually removed them from the remote, so that should fix it

from .Engine import *
from .Utils import *
from .PlotSetup import *
from .Plotter import *

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of your files are missing a final new line character

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for catching that. added a new commit fixing that

Comment on lines 32 to 34
SystemError: No plot variables specified
SystemError: Number of x limits does not match number of plot variables
SystemError: Number of linestyles does not match number of plot variables
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SystemError: No plot variables specified
SystemError: Number of x limits does not match number of plot variables
SystemError: Number of linestyles does not match number of plot variables
RuntimeError: No plot variables specified
RuntimeError: Number of x limits does not match number of plot variables
RuntimeError: Number of linestyles does not match number of plot variables

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, forgot to change the docstrings for that

if not plot_variables:
raise SystemError("No plot variables specified.")
plot_variables = config["VariableData"].get("plot_variables", "").split()
if len(plot_variables) == 0:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want, you can just write

Suggested change
if len(plot_variables) == 0:
if plot_variables:

An empty list evaluates to false.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yea thats right. I've been using so much C++ i forgot about those nice pythonic things. Changed in newest commit to this PR

elif __visit_script_file__ == __visit_source_file__:
raise RuntimeError("This file should not be run with VisIt, only Python!")

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file doesn't end with a newline

@@ -143,26 +67,25 @@ def main():
linestyles = None

if not len(linestyles) == len(plot_variables) and linestyles != None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little hard to follow these conditionals

also, comparisons with None should be done with is instead of ==

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in newest commit edcbb92

@ShaunFell ShaunFell changed the title Post-Processing Improvements Improvements: Post-Processing revamp Sep 5, 2024
Copy link

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@@ -5,6 +5,7 @@
hdf5_path = /home/hd/hd_hd/hd_pb293/WS_GRChombo/testing/GRMilijun_ProcaKerrBH_23320250/hdf5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an hardcoded path that users won't have access to

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the *.ini files are parameter files created by the user, as explained here

plot_files = [x for x in files if config["Header"]["plot_header"] in x]

#ensure plot files exist in provided directory
if len(plot_files) == 0:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, if you want to remove the len == 0, you can do it

files = glob.glob(filename_prefix+ "*.3d.hdf5")

#filter the file list to ensure regex-captured files are actual plot files
plot_files = [x for x in files if config["Header"]["plot_header"] in x]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use the filter function to express your intent more clearly

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thats probably better. added in latest commit

Close_Database(config) #includes error handling

print("All Done!")
sys.exit(0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

@ShaunFell ShaunFell merged commit 10aa7e4 into main Sep 7, 2024
22 checks passed
@ShaunFell ShaunFell deleted the feature/PostProcessingDocumentation branch September 7, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants