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

Update documentation for v0.7 release #266

Merged
merged 42 commits into from
Oct 7, 2024
Merged

Conversation

jank324
Copy link
Member

@jank324 jank324 commented Oct 3, 2024

Description

Updates the documentation to match Cheetah v0.7 for its release. A particular focus is put on the new broadcasting semantics, making sure that the entire documentation accurately reflects the latter.

Motivation and Context

Addresses #256.

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist

  • I have updated the changelog accordingly (required).
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.
  • I have reformatted the code and checked that formatting passes (required).
  • I have have fixed all issues found by flake8 (required).
  • I have ensured that all pytest tests pass (required).
  • I have run pytest on a machine with a CUDA GPU and made sure all tests pass (required).
  • I have checked that the documentation builds (required).

Note: We are using a maximum length of 88 characters per line.

@jank324 jank324 added the documentation Improvements or additions to documentation label Oct 3, 2024
@jank324 jank324 self-assigned this Oct 3, 2024
@jank324 jank324 linked an issue Oct 3, 2024 that may be closed by this pull request
@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

Should there be a vectorisation example notebook to explain how broadcasting works?

@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

For the simple.ipynb notebook, we will have to wait for #213 to be merged.

The gradientbased.ipynb notebook should be rewritten to take advantage of the vectorisation. I think this kind of goes hand in hand with rewriting the same example in the cheetah-demos repository.

@jank324 jank324 removed their assignment Oct 3, 2024
@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

The Accelerator, Error, Track Methods and Utils doc pages are empty 🤔

In Particles, only the docs for Beam appear. ParticleBeam and ParameterBeam are missing.

@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

I think in gradientbased.ipynb, where we show how to use the gradients for accelerator tuning, we can't really take advantage of the vectorisation, because we always want to use the gradient at the current magnet settings(?)

@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

Because their branches are already merged in here, this PR MUST only be merged into master AFTER #213 and #265.

@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

The Accelerator, Error, Track Methods and Utils doc pages are empty 🤔

In Particles, only the docs for Beam appear. ParticleBeam and ParameterBeam are missing.

So it turns out that rendering the docs does not work for files that have relative imports in them. We will have to keep that in mind for the future. For now, I returned all imports back to absolute imports (except for those in __init__.pys).

@jank324
Copy link
Member Author

jank324 commented Oct 3, 2024

Because their branches are already merged in here, this PR MUST only be merged into master AFTER #213 and #265.

‼️

@jank324 jank324 marked this pull request as ready for review October 3, 2024 21:52
@jank324 jank324 requested a review from cr-xu October 3, 2024 21:52
@cr-xu
Copy link
Member

cr-xu commented Oct 4, 2024

The Accelerator, Error, Track Methods and Utils doc pages are empty 🤔
In Particles, only the docs for Beam appear. ParticleBeam and ParameterBeam are missing.

So it turns out that rendering the docs does not work for files that have relative imports in them. We will have to keep that in mind for the future. For now, I returned all imports back to absolute imports (except for those in __init__.pys).

On a side note, what was the reason to switch to relative import? I think we started with having the full imports anyway.

@jank324
Copy link
Member Author

jank324 commented Oct 4, 2024

The Accelerator, Error, Track Methods and Utils doc pages are empty 🤔
In Particles, only the docs for Beam appear. ParticleBeam and ParameterBeam are missing.

So it turns out that rendering the docs does not work for files that have relative imports in them. We will have to keep that in mind for the future. For now, I returned all imports back to absolute imports (except for those in __init__.pys).

On a side note, what was the reason to switch to relative import? I think we started with having the full imports anyway.

I thought we started having the relative ones 😄. In general, I would argue that relative imports are better Python style.

The issue seems to be that Sphinx doesn't like them. The annoying bit there is that this error happens mostly silently. I only noticed it because parts of the docs were missing (specifically those from files that had at least one relative import in them). The only real clue that this was happening was in the build logs on readthedocs, where if you scroll through and look closely, you can find warnings like this one

WARNING: Failed to import accelerator.aperture.
Possible hints:
* KeyError: 'accelerator'
* ImportError: attempted relative import beyond top-level package

And then of course the fix is to make all imports absolute, e.g.

from  cheetah.themoduleyouwant import thethingyouwant

@cr-xu cr-xu merged commit b9374ae into master Oct 7, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update documentation for v0.7
2 participants