-
Notifications
You must be signed in to change notification settings - Fork 9
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
Expose marginal kwargs for moscot.temporal
and check for numeric type of temporal_key
#449
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #449 +/- ##
==========================================
+ Coverage 76.87% 77.12% +0.25%
==========================================
Files 40 40
Lines 3736 3760 +24
Branches 627 630 +3
==========================================
+ Hits 2872 2900 +28
+ Misses 591 588 -3
+ Partials 273 272 -1
|
Added a second, simpler way to compute prior marginals, thought this would fit well into this PR . |
src/moscot/_docs/_docs.py
Outdated
Specifies the left marginals. If | ||
- ``a`` is :class:`str` - the left marginals are taken from :attr:`anndata.AnnData.obs`, | ||
- if :meth:`~moscot.problems.base._birth_death.BirthDeathMixin.score_genes_for_marginals` was run and | ||
if `a` is `None`, marginals are computed based on a birth-death process as suggested in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't render nicely in the docs - missing indentation.
@@ -371,7 +371,7 @@ def push(self, *args: Any, **kwargs: Any) -> ApplyOutput_t[K]: | |||
%(scale_by_marginals)s | |||
|
|||
kwargs | |||
keyword arguments for :meth:`moscot.problems.CompoundProblem._apply`. | |||
keyword arguments for :meth:`~moscot.problems.CompoundProblem._apply`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you really wanna refer to the docs, apply
should be either made public, or you should explicitly tell sphinx to document this private method (currently links to nothing). Am OK with both options, the former is easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did an intermediate solution now , added TODO, and will think about the documentation of private functions later
src/moscot/problems/time/_mixins.py
Outdated
if key is not None and not is_numeric_dtype(self.adata.obs[key]): | ||
raise TypeError( | ||
"`temporal_key` has to be of numeric type." | ||
+ f"Found `adata.obs[{key!r}]` to be of type {infer_dtype(self.adata.obs[key])}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to use +
, also missing ' '
after ...numeric type.
Pet peeve: woul refer to the temporal key as Temporal key
, not `temporal_key`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by "also missing ' ' after ...numeric type"
…pe of `temporal_key` (#449) * make marginal_kwargs explicit in temporal problems * introduce check for numeric dtype in temporal mixin * add alternative way for marginal prior * adapt tolerances in tests * correct docs * fix bug * Fix math rendering * fix test Co-authored-by: Michal Klein <46717574+michalk8@users.noreply.github.com>
* fix jaxsampler * fix jaxsampler * fix jaxsampler * fix tests * add plot_convergence * remove jit from _compute_unbalanced marginals * fix sinkhorn_divergence * adapt tox.ini file * shape mismatch fixed without precommit * remove print statement * finish merge * adapt callbacks and rename tag `cost` to `cost_matrix` (#426) * rename tag cost to cost_matrix * fix renaming * [CI skip], adapt callback * incorporate requested changes * add test for quad custom callback * adapt kwargs for callback * fix handle_joint_attr * incorporate requested changes * Feature/correlation test (#423) * fix test in FGWProblem * add correlation test * add first test for correlation computation * add more tests * fix tests * add tfs to compute_feature_correlation * add testing for no nans in compute_feature_correlation * incorporate requested changes * fix docstring * fix sankey return statement (#428) * fix sankey return statement * adapt test * adapt return_fig * Bump version: 0.1.0 → 0.1.1 * fix return statements * add save tests * fix return type in mpl (#432) * fix return type in mpl * change import acronyms * fix tests * Simplify linear operator (#431) * Simplify linear operator * Simplify `align`, fix test * Explicitly jit the solvers (#433) * Feature/interpolate colors sankey (#434) * fix return type in mpl * change import acronyms * fix tests * add interpolation option to sankey * add test to interpolate color * define colors for pull/push` * adapt tests * introduce axes in mpl.push/pull * incorporate requested changes * change default color * adapt plotting * introduce scaling * fix scale * make start/end categorical in plot * regenerate images * Remove `FGWSolver` (#437) * Remove `FGWSolver` * Fix `tox.ini` * Fix wrong shape check * Use pure GW in generic solver * Update tests * fix bug in SinkhornProblem (#442) * fix bug in SinkhornProblem * fix tox.ini * fix pre commits * make push/pull always use source/target (#443) * make push/pull always use source/target * fix bug in StarPolicy _apply * adapt plotting to source/target * fix strip plotting in sankey (#445) * fix strip plotting in sankey * simplify code * Feature/spearman correlation (#444) * add spearman correlation * add tests * adapt tests * Delete logo.png * Feature/plot order (#453) * make push/pull plot in good order * [CI skip], try setting adata.uns color explicitly * [CI skip], fix copying of adata * fix pre commits * fix bug * Expose marginal kwargs for `moscot.temporal` and check for numeric type of `temporal_key` (#449) * make marginal_kwargs explicit in temporal problems * introduce check for numeric dtype in temporal mixin * add alternative way for marginal prior * adapt tolerances in tests * correct docs * fix bug * Fix math rendering * fix test Co-authored-by: Michal Klein <46717574+michalk8@users.noreply.github.com> * adapt plot_convergence (#454) * Bug/docs generic analysis mixin (#455) * adapt plot_convergence * remove temporal-alluding docs in generic analysis mixin * Docs/improvements (#456) * adapt plot_convergence * remove temporal-alluding docs in generic analysis mixin * docs suggestions * remove uns_key from set_plotting_vars (#458) * resolve `fig referenced before assignment` (#460) * move generic mixins tests to problems` (#461) * Tests/spatiotemporalproblem (#464) * add more tests for spatiotemporalProblem * move some functions from TemporalProblem to TemporalMixin * add tests LineageProblem * fix tests * Feature/move taggedarray (#457) * adapt plot_convergence * remove temporal-alluding docs in generic analysis mixin * docs suggestions * move tagged array * move taggedarray back to solvers * add marginal_kwargs to prepare method of TemporalNeuralProblem * fix to scaling in * Revert "fix to scaling in" This reverts commit 0a6f7db. * fix to scaling argument in marginal_kwargs * updated conditional not pipeline * merge into condot branch * incoporated comments * incoporated comments * incoporated comments * removed new_adata for push/pull * [ci skip] start docs * added temporal neural test * [ci skip] continue docs * continue docs * continue docs * change validation epsilon * fixed error when not computing wasserstein baseline * fixed error when not computing wasserstein baseline * correct typo * fix bug * added neural tests * [ci skip] draft CondNeuralOutput * include CondDualPotentials and CondDualSolver * fixes to main merge * fix test_cell_transition_subset_pipeline * fix tests * update conditionalDualPotentials * update conditionalDualPotentials * fix most pre-commit hooks and fix tests * fix pandas version to <2.0 * fix tests for non-conditional solvers * continue * fix * continue fixing * fix ICNN setup * fix tests * swap role of f and g, such that push/pull is correct again * [ci skip] restructure to include more general neural solvers * [ci skip] restructure ICNNs to allow passing instances of ICNN * adapt tests * Filled in Monge Gap structure * Added Monge Gap paper to documentation * Ammend PointCloud Import * Update _utils.py Ammend PointCloud import * Solve compatibility issue with ProblemKind * Solve missing Import * Fix call to deprecated function * Fix style and comment issues * add callback, swap f & g * add callback, swap f & g * add callback, swap f & g * intermediate save * intermediate save * intermediate save * [ci skip] fix merge conflicts * resolve conflict * remove pairwise policy * add neural dependencies * add neural dependencies * add flax * fix _call_kwargs * fix marginal kwargs * remove monge gap solver * clean condneuralsolver * [ci skip] introduce new data container for joint neural problems * add conditions in distirbutioncontainer * resolve unfreeze/freeze * enable pretraining and weight clipping * make dicts compatible with older python versions * resolve precommit errors partially * resolve precommit errors partially * adapt tests * [ci skip] draft unbalancedNeuralMixin * [ci skip] fix naming of posterior marginals * [ci skip] add MLP_marginals * adapt neural output to incorporate learnt rescaling functions * fix _solve in neuraldualsolver * incorporate feedback * fix distributioncollection class * unify _split_data * fix tests * fix some precommit hooks * make neural dependencies optional * make neural dependencies optional * delete old files * adapt pyproject.toml * adapt pyproject.toml * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [ci skip] adjust _format_params * adapt neuraldualsolver to be more similar to ott-jax * adapt neuraldualsolver * TODO: make JaxSampler return conditions * add basic neural test * [ci skip] intermediate save * adapt neuraldualsolver and finish tests for neural backend * [ci skip] TODO: re-iterate on initialisation of neural solver * adapt distributioncontainer * fix dict bug * resolve passing of arguments in solver call methods * [ci skip] adapt `solve` in `CondOTProblem` * adapt tests and valid loader conditions * adapt neural backend tests * fix mypy errors * make basesolveroutput to basediscretesolveroutput * move `to` to BaseSolverOutput` " * adapt transport_matrix docs * adapt transport_matrix docs * adapt tests * adapt tests * update unbalancedness mixin * use implementation from moscot * uncomment unused code * before passing states to loss-fn * intermediate save * adapt neuraldualsolver * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * resolve some / not all pre commit errors * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * (wip): tests run, code swapped out for now * (wip): `NeuralSolver`s implemented minus quad/linear * (wip): begin more generic problem * (wip): more refactoring to pass arguments to GENOT * (chore): remove more kantorovich * (chore): update branch to moscot neural + first test moving to solving * (fix): split data remains in numpy * (fix): push/pull api * (fix): make push test work * (feat): allow for custom optimziers * (chore): remove unclear test * (refactor): change to composition API * (refactor): start towards model-specific problems * (chore): clean up all unnecessary classes * (chore): updating to moscot latest * Merge branch 'main' into ig/neural_solvers * (chore): remove (hopefully) final ICNN vestiges * (chore): more cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * (fix): pass pre-commit hooks * (chore): remove duplicatec docs * (chore): add torch for testing * (fix): add ott jax branch as dep * (fix): repo name * (chore): remove unbalanced, update api, fix tests + drive by typing fix * (feat): first pass at neural mixin * (chore): add my name to todos * (fix): conditions left out if not necessary * (feat): logs and fix conditional attr * (fix): add `seed` to call_kwargs so reproducibility works * (chore): remove `is_conditional` business * (fix): create hidden dims arg for velocity field * (chore): raise not implemented error for `pull` * (fix): default args * (fix): add explicit policy * (fix): allow iteration to continue * (chore): add star policy to GENOT * (chore): notebooks * (chore): remove deps * (chore): remove unnecessary spaces * (chore): simplify quad handling * (fix): need to require `optax`/`flax` * (fix): use `ott-jax[neural]` * (chore): fix docs * (fix): small test fixes * (chore): small notebook changes * (fix): broken link in citation * (chore): make notebook dependent on ci * (fix): small todos just to push something * (fix): variable is a string * (fix): pass environment variable to tox * (fix): actually pass through * (fix): hidden dims ci * (fix): re-add notebook * (chore): make`recall_target` and `aggregate_to_topk` * (chore): fix default arguments * (chore): `project_transport_matrix` -> `project_to_transport_matrix` * (fix): remove dead `NeuralAnalysisMixin` code * (feat): allow custom `data_match_fn` * (fix): inherit from `MutableMapping` instead of `dict` * (Fix): docs * (fix): notebooks * (fix): docs reference * (fix): remove `attr` * (fix): erroneous change * (fix): remove empty * (fix): notebooks again? * (chore): ok? --------- Co-authored-by: Dominik Klein <dominik.klein@helmholtz-munich.de> Co-authored-by: Dominik Klein <domin.klein@gmail.com> Co-authored-by: AlejandroTL <alejandrotejada10@gmail.com> Co-authored-by: michalk8 <46717574+michalk8@users.noreply.github.com> Co-authored-by: lucaeyring <luca.eyring@googlemail.com> Co-authored-by: gocato <104785310+gocato@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Resolves #446 , #448