Skip to content

Conversation

@andrea-petrocchi
Copy link
Contributor

Copy link
Member

@fangohr fangohr 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.

Three items of feedback:

  1. It appears the actual change to the notebook is missing:
Screenshot 2025-12-18 at 12 56 57
  1. The change in the metadata (change of python version used) should not be in the commit change, I think.

  2. For this tiny change I would suggest we merge (once resolved), and force push the tag 0.11.0 again to get an update of the webpages. We may have more small changes on the wording of the notebooks, which also do not warrant a new version I think. If we do this, we don't need add 58.fixed.mi I think. (Do you agree?)

@github-project-automation github-project-automation bot moved this from In review / waiting to In progress in MaMMoS project management MPSD Dec 18, 2025
@andrea-petrocchi
Copy link
Contributor Author

andrea-petrocchi commented Dec 18, 2025

I run the whole notebook and pushed the changes.

  1. The right changes are in there for me:
    tmp
    Anyway, the notebook in the branch seems to be correct with the changes: https://github.com/MaMMoS-project/mammos/blob/omp-num-threads/examples/spindynamics-temperature-dependent-parameters.ipynb
  2. I can delete that change.
  3. The notebook requires a patch in mammos-spindynamics.

For me this would be the best to-do-list:

  1. I have already pushed that fix to main but I have not published the package.
  2. When PR 55 is also approved, I would publish mammos-spindynamics 0.3.1.
  3. Then I would set mammos-spindynamics 0.3.1 in this PR and publish a patch

If you don't agree, we can push this PR directly as well (but people running the notebook might see the list in the .info() in different order).

@andrea-petrocchi andrea-petrocchi merged commit 7e0e551 into main Dec 18, 2025
9 of 12 checks passed
@andrea-petrocchi andrea-petrocchi deleted the omp-num-threads branch December 18, 2025 14:54
@lang-m
Copy link
Member

lang-m commented Jan 5, 2026

For this tiny change I would suggest we merge (once resolved), and force push the tag 0.11.0 again to get an update of the webpages. We may have more small changes on the wording of the notebooks, which also do not warrant a new version I think. If we do this, we don't need add 58.fixed.mi I think. (Do you agree?)

Please never force-push a version tag to rebuild the webpages.

Instead, manually trigger the documentation action on main: https://github.com/MaMMoS-project/mammos/actions/workflows/documentation.yml and tick the box to deploy changes:

image

To get updated notebooks for binder only force-push the latest tag.

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Change OMP_NUM_THREADS in mammos demonstrator example

4 participants