Skip to content

Playing with the Hemato notebook#20

Draft
elvijs wants to merge 2 commits into
masterfrom
elvijs/playing-with-hematopoiesis
Draft

Playing with the Hemato notebook#20
elvijs wants to merge 2 commits into
masterfrom
elvijs/playing-with-hematopoiesis

Conversation

@elvijs

@elvijs elvijs commented Oct 15, 2020

Copy link
Copy Markdown
Contributor

Just making some small changes to the notebook as I'm familiarising myself with the codebase. Not convinced all the changes are useful, so only making this a draft PR for now.

Diff viewer isn't great for notebooks, see the updated notebook here and compare with the current version here.

Summary of changes:

  • Creating a master data frame with all the data in one place
  • Moving plotting to seaborn. This simplifies some of the scatter plots substantially
  • Type annotations and other minor readability improvements

TODO:

  • some of the details have gone missing in the last scatter plot - that still needs to be rectified. We should add a subtitle explaining what the different pieces do.

@alexisboukouvalas

Copy link
Copy Markdown
Contributor

@elvijs how about we use jupytext? We could push both the .py and .pynb files or just the former. This would allow us to more easily review changes to the notebook. What do you think?
@sumonahmedUoM do you have a view on this?

@alexisboukouvalas alexisboukouvalas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New plots look much nicer and more intelligible!
I am wondering if we can get rid of the warnings -- perhaps redirect them to a log file?

@elvijs

elvijs commented Oct 18, 2020

Copy link
Copy Markdown
Contributor Author

@elvijs how about we use jupytext? We could push both the .py and .pynb files or just the former. This would allow us to more easily review changes to the notebook. What do you think?
@sumonahmedUoM do you have a view on this?

I like that a lot. Let me prototype in a separate PR; we can then come back to this one when it's easier to review :)

@elvijs

elvijs commented Oct 18, 2020

Copy link
Copy Markdown
Contributor Author

New plots look much nicer and more intelligible!
I am wondering if we can get rid of the warnings -- perhaps redirect them to a log file?

Yeah, that sounds nicer, let me prototype that in a separate PR.

@sumonahmedUoM

Copy link
Copy Markdown

@elvijs how about we use jupytext? We could push both the .py and .pynb files or just the former. This would allow us to more easily review changes to the notebook. What do you think?
@sumonahmedUoM do you have a view on this?

I like that a lot. Let me prototype in a separate PR; we can then come back to this one when it's easier to review :)

Yes, in the meantime I can learn something about jupytext :)

@sumonahmedUoM

sumonahmedUoM commented Oct 20, 2020

Copy link
Copy Markdown

New plots look much nicer and more intelligible!
I am wondering if we can get rid of the warnings -- perhaps redirect them to a log file?

Yeah, that sounds nicer, let me prototype that in a separate PR.

Cau we use the following to get rid of unwanted messages? @elvijs @alexisboukouvalas

tf.logging.set_verbosity(tf.logging.ERROR) # Suppresses output logs from tensorflow

@elvijs

elvijs commented Oct 21, 2020

Copy link
Copy Markdown
Contributor Author

New plots look much nicer and more intelligible!
I am wondering if we can get rid of the warnings -- perhaps redirect them to a log file?

Yeah, that sounds nicer, let me prototype that in a separate PR.

Cau we use the following to get rid of unwanted messages? @elvijs @alexisboukouvalas

tf.logging.set_verbosity(tf.logging.ERROR) # Suppresses output logs from tensorflow

This would suppress the TF logs instead of redirect to a temporary file. This is quicker and simpler, but can hide genuine issues.

At a quick glance, it looks like there are two types of messages:

  1. TF warnings relating to outdated use of TF1 in GPflow1, see below. I think these might go away once Version 2.0 #13 is merged in.
WARNING:tensorflow:From /home/elvijs/dev/BranchedGP/.oldvenv/lib/python3.7/site-packages/gpflow/kernels.py:215: The name tf.matrix_diag is deprecated. Please use tf.linalg.diag instead.
  1. Some sort of convergence message. Not sure what actually produces this, does this come from somewhere BranchedGP @sumonahmedUoM ?
INFO:tensorflow:Optimization terminated with:
  Message: b'CONVERGENCE: REL_REDUCTION_OF_F_<=_FACTR*EPSMCH'
  Objective function value: 118.157363
  Number of iterations: 28
  Number of functions evaluations: 32

@elvijs elvijs force-pushed the elvijs/playing-with-hematopoiesis branch from 0e499ca to 56fc6d1 Compare November 1, 2020 17:18
@elvijs elvijs force-pushed the elvijs/playing-with-hematopoiesis branch from 8e3612c to 3db5517 Compare November 15, 2020 17:17
@alexisboukouvalas

Copy link
Copy Markdown
Contributor

@elvijs please resolve the conflicts - perhaps after merging the pytest and TF version PRs?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants