-
Notifications
You must be signed in to change notification settings - Fork 78
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
Tackle Typing and Linting Errors #379
Draft
gileshd
wants to merge
36
commits into
main
Choose a base branch
from
ghd/typing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes: - Replace `jnp.ones` input with `jr.normal`. - Reduce size of hidden state to 3. - Remove unused `datetime` import and commented lines. Fundamentally the problem is that the solve step can be unstable. This does not resolve that but instead chooses a set up which is less vulnerable to the instability.
- Rename to PRNGKeyT to differentiate from `jax.random` function - Change type to Array - see https://jax.readthedocs.io/en/latest/jep/9263-typed-keys.html#type-annotations-for-prng-keys
gileshd
force-pushed
the
ghd/typing
branch
3 times, most recently
from
September 23, 2024 12:03
c11f50f
to
685028a
Compare
This stops type-checkers worrying about escape-sequences.
Attributes of `HHMParameterSet` and `HMMPropertySet` are changed to read-only. This change means that instances of `NamedTuple` will implement these protocols.
gileshd
force-pushed
the
ghd/typing
branch
3 times, most recently
from
September 24, 2024 13:50
5b4f00a
to
55b17f9
Compare
- Add annotations to the *GaussianHMMEmssions classes - Apply some small formatting changes to reduce line length in places
- Add annotations to the *GaussianMixtureHMMEmssions classes
Major changes: - Fix an error in `_compute_all_transition_probs` which caused the return array to be too short. - The `filtered_probs` were being truncated twice instead of once. - Replace `jaxtyping.Int` with `dynamax.typing.IntScalar` or `int` - this reflects when integer scalar arrays are accepted - `jaxtyping.[Dtype]` cannot be used directly for type checking instead they must be used as part of an array. - Fix the shape of `transition_matrix`: - if transition_matrix has a leading timestep axis it should be of length T-1 not of length T. - Add annotation indicating that `transition_matrix` is an optional argument - Raise ValueError when neither `transition_matrix` or `transition_fn` provided.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR is a Work In Progress.
Fix the current errors with type hints and some common linting errors.
Details
Commits
A brief categorisation of the commits:
Address review comment "Incorrect typing"
Linting Errors:
Fix Type Errors:
jr.PRNGKey
which, as a function, isn't suitable for type hinting here.jaxtyping.Array
as per suggestion heredynamax.types
module that defines a type for a key - I update this definition as per suggestion linked above.dynamax.types.PRNGKeyT
(which is an alias forjaxtyping.Array
) and sometimes directly asjaxtyping.Array
.Enable runtime type checking:
Unrelated Changes: