Skip to content

Conversation

@Gbowker
Copy link
Collaborator

@Gbowker Gbowker commented Dec 3, 2025

Demo which runs tensile simulations at 0, 45 and 90 to cube texture.

  • Notebook which plots the data.
  • Remove resources to make machine-agnostic
  • Add demo to manifest so it can be run from CLI.

@Gbowker Gbowker requested a review from a team as a code owner December 3, 2025 11:35
@Gbowker Gbowker self-assigned this Dec 3, 2025
@Gbowker Gbowker added the enhancement New feature or request label Dec 3, 2025
@dkfellows
Copy link
Collaborator

Largely looks fine to me, though I emphasise that I'm no expert on the scientific content.

Definitely minor point

I think things like:

Path(f"{workflow_path}/execute/t_1/e_0/pole_figure.png")

can instead (because workflow_path is already a Path) be written as:

workflow_path / "execute/t_1/e_0/pole_figure.png"

I believe that those two are equivalent.

I would not regard that as a reason to reject a PR.

@Gbowker
Copy link
Collaborator Author

Gbowker commented Dec 4, 2025

Thanks @dkfellows ! Any review is a good review!
I acknowledge that this requires several improvements in the notebook to make it more streamlined and easy to understand as a demo and am happy to make any changes like this to do so.

Guy

Copy link
Collaborator

@dkfellows dkfellows left a comment

Choose a reason for hiding this comment

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

Code looks acceptable. I don't know the science to tell for sure, but there's nothing in there that looks wildly off.

@aplowman
Copy link
Collaborator

Looks good @Gbowker. In the notebook, would you be able to add a link to the demo workflow template on the docs site that you used to generate the workflow (in the format https://docs.matflow.io/stable/reference/workflows.html#tension-damask-al, where you modify tension-damask-al)? And a couple of sentences on what the R-values should look like for the different loading directions? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants