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

Implement plotObservedVsSimulated() #945

Merged
merged 64 commits into from
Jul 18, 2022

Conversation

IndrajeetPatil
Copy link
Member

@IndrajeetPatil IndrajeetPatil commented May 13, 2022

TODO

Features that need to be implemented in tlf, and thus should not block this PR from being merged:

Related to #674

Discussed in #809

Closes #804
Closes #924
Closes #1010
Closes #1011
Closes #1012

@IndrajeetPatil IndrajeetPatil marked this pull request as draft May 13, 2022 15:58
@IndrajeetPatil IndrajeetPatil changed the title Implement plotObservedVsSimulated() WIP: Implement plotObservedVsSimulated() May 13, 2022
@IndrajeetPatil
Copy link
Member Author

@msevestre, @Yuri05 This PR is ready for a review. Thanks.

@IndrajeetPatil

This comment was marked as outdated.

@IndrajeetPatil
Copy link
Member Author

Observed data is plotted with error bars now.

Note that the error bar is missing in the plot below for some data points because the error value is 0.

obs_vs_pred

@Yuri05
Copy link
Member

Yuri05 commented Jul 15, 2022

Should we display the error bars in the same way as usually?
grafik

@IndrajeetPatil
Copy link
Member Author

Should we display the error bars in the same way as usually?

This would be easy to do by setting height to some non-zero value in geom_errorbarh().

I personally find those vertical edges distracting. Plus, they don't convey any additional information, so it's just adding clutter to an already busy plot. So I'd prefer not adding the edges.

I'm curious to hear what others think.

@Yuri05
Copy link
Member

Yuri05 commented Jul 15, 2022

they don't convey any additional information, so it's just adding clutter to an already busy plot. So I'd prefer not adding the edges.

In general, I agree.
However we display error bars always like this (example) and looking at the plots by others - same in the most cases (not always though)

@IndrajeetPatil
Copy link
Member Author

IndrajeetPatil commented Jul 15, 2022

@Yuri05 Does this mean that we should also be displaying error bars with caps in profile plots?

Currently, we have error bars without the caps:

image

@PavelBal
Copy link
Member

Does this mean that we should also be displaying error bars with caps in profile plots?

Hmm yes, I did not realize this but yes, it is usual to display caps for error bars.

@Yuri05
Copy link
Member

Yuri05 commented Jul 15, 2022

Does this mean that we should also be displaying error bars with caps in profile plots?

I would say yes :)

@IndrajeetPatil
Copy link
Member Author

Okay, I've created an issue it tlf to add caps to error bars: Open-Systems-Pharmacology/TLF-Library#333

Once it is added there, it will automatically be included in the ospsuite plots as well.

@IndrajeetPatil
Copy link
Member Author

@PavelBal and @Yuri05 I've created issues for features that need to be implemented in tlf, and thus should not block this PR from being merged:

I don't want to keep this pending anymore, since this is blocking me from working on #1002.

@Yuri05
Copy link
Member

Yuri05 commented Jul 18, 2022

@PavelBal and @Yuri05 I've created issues for features that need to be implemented in tlf, and thus should not block this PR from being merged:

* [Supporting fold distance in linear scale TLF-Library#332](https://github.com/Open-Systems-Pharmacology/TLF-Library/issues/332)

* [Add caps to error bars TLF-Library#333](https://github.com/Open-Systems-Pharmacology/TLF-Library/issues/333)

I don't want to keep this pending anymore, since this is blocking me from working on #1002.

fine for me

@IndrajeetPatil
Copy link
Member Author

Thanks, Yuri.

@PavelBal Let me know if you have any other comments over and above the ones that have already been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants