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

Cavitation Model Example And Example Warning Fix #2102

Merged
merged 38 commits into from
Oct 20, 2023
Merged

Conversation

ansaboutin
Copy link
Contributor

@ansaboutin ansaboutin commented Oct 6, 2023

Created a new PyFluent example following the Chapter 22: Modeling Cavitation Fluent example.

A few tui commands remain. Equivalent settings commands do not exist.

examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
@seanpearsonuk
Copy link
Collaborator

Hi @ansaboutin Re the question about replacing TUI commands with settings API commands, please see this merged PR by @mkundu1 : #2045, which automates the discovery of such translations: "advice will be printed if an equivalent settings API exists".

Copy link
Member

@raph-luc raph-luc left a comment

Choose a reason for hiding this comment

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

There are too many code comments (compare this with other examples in the repo):

Screenshot example

image

I would suggest transforming almost all of the code comments into text (everywhere in this example, not only the screenshot above), and moving them to appropriate headings (or alternating between code and text as you see fit). I mentioned a few instances below.

See this for more details on how to switch between text and code: https://sphinx-gallery.github.io/stable/tutorials/plot_parse.html (download python source code at the bottom)

examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
Copy link
Member

@raph-luc raph-luc left a comment

Choose a reason for hiding this comment

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

If the description for each section is going to be in a single block of text, then the text needs to be revised to be more readable (I suggested a few changes as examples). Breaking the longest text blocks into paragraphs may help as well.

In addition, if you think it would be more readable, feel free to consider breaking the larger code blocks into smaller parts, and spreading the text in between code blocks. For reference, see the link I sent above with information on how to more easily switch between text and code blocks.

examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
Copy link
Member

@raph-luc raph-luc left a comment

Choose a reason for hiding this comment

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

Looks much better, thanks @ansaboutin, some additional suggestions below

As in my last suggestion, I think outputting the images to files, that are being shown in the example, is a good idea.

examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Show resolved Hide resolved
examples/00-fluent/modeling_cavitation.py Outdated Show resolved Hide resolved
@ansaboutin ansaboutin merged commit 6d0b080 into main Oct 20, 2023
18 checks passed
@ansaboutin ansaboutin deleted the feat/new_example branch October 20, 2023 18:18
raph-luc added a commit that referenced this pull request Oct 25, 2023
* New example.

* Upto line 152.

* Forgot .py extension

* Solution complete.

* commands completed.

* Text updated.

* Pictures added.

* Set the vaporization pressure to comment.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removed show_gui.

* Moved sphinx into code block.

* Removed momentum from the object train.

* Residuals changed to tui.

* cwd removed.

* cwd removed from compressible flow.

* Removed pd.concat().

* Removed local requirements.

* Image adjustments.

* Pa changes to kPa.

* Thumbnail updates.

* Concat dataframe if it is not empty.

* Code comments removed.

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Adjusted for easier reading.

* Removed save_path.

* cav.msh -> cav_file

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* Update examples/00-fluent/modeling_cavitation.py

Co-authored-by: Raphael Luciano <[email protected]>

* Some additional suggestions below.

* Read mesh to text not comment.

* Changed to sentence case.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Raphael Luciano <[email protected]>
raph-luc added a commit that referenced this pull request Oct 26, 2023
* Fix vale warnings (#2139)

* fix tensor type for displacement variable (#2145)

* Fix set_state implementation for command argument instance. (#2147)

* Update flobject.py (#2148)

* SVAR Doc (#1635)

* Test to catch Watchdog launch errors, and improved Watchdog behavior on Windows (#2144)

* Cavitation Model Example And Example Warning Fix (#2102)

* Add type annotations for some modules under services (#2108)

* More robust Windows launch command for Watchdog (#2167)

* Making h5py an optional dependency, not installed by default (#2171)

* Expose settings root like in pyconsole. (#2149)

* Remove timeout loop in FluentConnection (#2126)

* Fix SVAR doc (#2172)

---------

Co-authored-by: Mainak Kundu <[email protected]>
Co-authored-by: Oleg Chernukhin <[email protected]>
Co-authored-by: Prithwish Mukherjee <[email protected]>
Co-authored-by: Harshal Pohekar <[email protected]>
Co-authored-by: Aseem Jain <[email protected]>
Co-authored-by: Prithwish Mukherjee <[email protected]>
Co-authored-by: Adam Boutin <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

Examples have some Python warnings in the output
5 participants