Skip to content

Adding TechDemo 25: Cardiovascular Stent Simulation #15

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 54 commits into from
Closed

Adding TechDemo 25: Cardiovascular Stent Simulation #15

wants to merge 54 commits into from

Conversation

@clatapie
Copy link
Collaborator

@clatapie clatapie commented Nov 24, 2022

Closes #10

@clatapie clatapie requested a review from RobPasMue November 24, 2022 09:54
@clatapie clatapie self-assigned this Nov 24, 2022
@clatapie clatapie linked an issue Nov 24, 2022 that may be closed by this pull request
TechDemo 25: Cardiovascular Stent Simulation #10
Closed
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 24, 2022
@clatapie clatapie requested a review from germa89 November 25, 2022 17:08
germa89
germa89 approved these changes Nov 28, 2022
View reviewed changes
Copy link
Contributor

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

I just realised it is a draft xD...

@germa89
Copy link
Contributor

germa89 commented Nov 28, 2022

@clatapie ping also Kathy when the main of the text is ready.

@germa89
Copy link
Contributor

germa89 commented Dec 13, 2022

I'm removing myself from reviewers, re-add me when this draft is ready to be reviewed.

@github-actions github-actions bot added the dependencies Related with project dependencies label Dec 27, 2022
RobPasMue
RobPasMue reviewed Jan 10, 2023
View reviewed changes
@github-actions github-actions bot added the maintenance Package and maintenance related label Jan 10, 2023
RobPasMue
RobPasMue reviewed Jan 10, 2023
View reviewed changes
@RobPasMue
Copy link
Member

RobPasMue commented Jan 10, 2023

Starting final review! :)

RobPasMue
RobPasMue approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

LGTM! Just one minor comment. I'm no expert on the techdemo itself, so I trust your implementation. Thanks @clatapie, great work!!!

@RobPasMue RobPasMue marked this pull request as ready for review January 10, 2023 14:21
clatapie and others added 2 commits January 10, 2023 15:26
@clatapie
Updating the python file
c1eecac
@clatapie @RobPasMue
Update requirements/requirements_doc.txt
54ad689 
Co-authored-by: Roberto Pastor Muela <roberto.pastormuela@ansys.com>
@clatapie clatapie requested a review from germa89 January 10, 2023 14:30
germa89
germa89 approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

Nice work!

Let's see how it looks once it has been deployed.

Some comments:

  • Some of the images are very small.
  • you are attaching db files, But I'm not sure if you are using them all. For example stent.db.

doc/source/index.rst
Comment on lines +252 to +270
.. ##########
.. raw:: html

<div class="sphx-glr-thumbcontainer" tooltip="Technology Showcase Example 15: Calibrating and Validating a Hyperelastic Constitutive Model">

.. only:: html

.. image:: technology_showcase_examples/images/thumb/sphx_glr_tse-015-calvalhyper_thumb.gif
:alt: Calibrating and Validating a Hyperelastic Constitutive Model

:ref:`sphx_glr_ex_15-teccalvalhyper.rst`

.. raw:: html

<div class="sphx-glr-thumbnail-title">Calibrating and Validating a Hyperelastic Constitutive Model</div>
</div>

.. #########
Copy link
Contributor

@germa89 germa89 Jan 10, 2023

Choose a reason for hiding this comment

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

I guess this is generated by sphinx?

Copy link
Collaborator Author

@clatapie clatapie Jan 10, 2023

Choose a reason for hiding this comment

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

I extended it: I don't have an example repository so I don't generate the Sphinx gallery like PyMAPDL or PyMAPDL-examples do. Let me know if it's not good practice!

@clatapie
Copy link
Collaborator Author

clatapie commented Jan 11, 2023 •
edited
Loading

Hello @PipKat,
I'm pinging you to get your feedbacks on this new repository and the new TechDemo example.
Please, feel free to let me know if you want me to make any changes.

@clatapie clatapie requested a review from PipKat January 11, 2023 10:55
akaszynski
akaszynski suggested changes Jan 11, 2023
View reviewed changes
Copy link

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

There is a bunch of generated content here that shouldn't be included in this PR.

Let's remove everything that's generated and instead make it procedural by using sphinx-gallery or myst-nb as used in pygeometry.

@clatapie clatapie closed this Jan 12, 2023
@germa89 germa89 deleted the doc/adding_techdemo25 branch February 26, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@germa89 germa89 germa89 approved these changes

@RobPasMue RobPasMue RobPasMue approved these changes

@PipKat PipKat Awaiting requested review from PipKat

+1 more reviewer

@akaszynski akaszynski akaszynski requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@clatapie clatapie

Labels

dependencies Related with project dependencies documentation Improvements or additions to documentation maintenance Package and maintenance related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TechDemo 25: Cardiovascular Stent Simulation

5 participants

@clatapie @germa89 @RobPasMue @akaszynski