Responsiveness logic #25

lauraporta · 2023-02-14T03:26:17Z

No description provided.

…veness

…ical calculations (last one stil; buggy)

lauraporta · 2023-03-02T17:09:13Z

What there is in this PR?

Methods to analyse neurons' responsiveness in the FrequencyAnalysis class
Improvements in PhotonData object
Improved tests for PhotonData and new tests for FrequencyAnalysis classes
A description of the structure of raw and processed data in README.md

Let me know if the description in the README.md file is not sufficient to understand the PR!

JoeZiminski

Hey excellent work @lauraporta! The code is super clear and easy to review, and it runs very smoothly. It is also very easy to test now with the new features (e.g. setup_for_testing.py). I've added some suggestions to discuss but there are no hard changes requested, please let me know if anything isn't clear. Looking forward to the dashboard!

README.md

rsp_vision/main.py

rsp_vision/objects/photon_data.py

JoeZiminski · 2023-03-14T16:32:22Z

rsp_vision/objects/photon_data.py

        self.n_sessions = data_raw.frames.shape[0]
        self.n_roi = data_raw.frames[0].shape[0]
        self.n_frames_per_session = data_raw.frames[0].shape[1]
-        self.day_stim = data_raw.day["stimulus"]
+        self.day_stim = data_raw.day["stimulus"]  # seems useless


for these variables, I get IDE style warning that all class attributes should be defined in init (i.e. initialise as None). Unfortunately, the IDE complains even if putting a type hint there rather than initialising. It does seem worth doing this even though annoying.

related to this, because this function contains routines that make assumptions based on the experimental setup (e.g. baseline before and after triggers) e.g. self.n_all_triggers - 2 * self.n_session_boundary_baseline_triggers ) * self.n_sessions. Maybe this function can be factored out somewhere, if it may change depending on experimental setup (e.g. there could be multiple versions). Then, it can return all these variables in the init where they are saved as class attributes

I see. This is a complex refactor of the class PhotonData. However to do so I need more information on how to handle new types of raw_data that come from multiple days as well. I will do In a separate PR.
New issue opened #37

JoeZiminski · 2023-03-14T16:36:08Z

rsp_vision/objects/photon_data.py

@@ -72,13 +78,10 @@ def set_general_variables(self, data_raw):
            The raw data object from which the data will be extracted
        """
        self.screen_size = data_raw.stim[0]["screen_size"]
-        self.is_cell = data_raw.is_cell
-        self.day_roi = data_raw.day["roi"]
-        self.day_roi_label = data_raw.day["roi_label"]
        self.n_sessions = data_raw.frames.shape[0]
        self.n_roi = data_raw.frames[0].shape[0]
        self.n_frames_per_session = data_raw.frames[0].shape[1]


(if my understanding is correct, and each element of frames is a session), just checking, can it always be assumed the number of frames will be the same across sessions? (e.g. if one day, things go wrong halfway through the session etc)

Actually yes the number of frames are the same across sessions if everything turned out well. I know this a priory actually, I could just add a check.
#37

JoeZiminski · 2023-03-21T13:43:16Z

tests/test_integration/conftest.py

+
+
+@pytest.fixture
+def get_sf_tf_instance(get_config, get_data_raw_object):


This is a small point, but I believe the tests are all currently interlinked. e.g. create raw data object (is tested) -> from this make PhotonData (is tested) -> from this make Frequency results. This makes a lot of sense but creates some interdependency between tests (e.g. if something breaks in PhotonData is will break all tests after this). You could consider splitting this interdependency by creating a completely new fake photondata.signal table to feed into FrequencyAnalysis. However, this will be more work and in practice if PhotonData does break then you'd just fix it and re-run tests. But just something to note

I see what you mean. Another idea is to abracting more the analysis methods, make them static, in a way that they work independently from which kind of object they are handling under the hood. This is a bit hard and I didn't figured out yet a good way to do it, but I think that you are right. It is worth keeping this as an objective.

JoeZiminski · 2023-03-21T13:45:00Z

tests/test_integration/test_sf_tf.py

+        window_mask_baseline,
+    ) = sf_tf.get_response_and_baseline_windows()
+
+    assert (


would it also be possible / does it also make sense to check all variables, and all arrays elements by element? is it possible that the len() might be the same but somehow actual values are different?

It is possible and you are right 🥹 But I didn't came up with an elegant way to do this without hard coding the numbers.

JoeZiminski · 2023-03-21T13:51:26Z

tests/test_integration/test_sf_tf.py

+    sf_tf.calculate_mean_response_and_baseline()
+    p_values = sf_tf.nonparam_anova_over_rois()
+
+    decimal_points = 3


I think this onwards could be refactored the below and be equivilent (in other tests too):

p_values = list(p_values.values()) p_values_seed_101 = np.array([0.055, 0.473, 0.324, 0.127, 0.653]) assert np.allclose(p_values, p_values_seed_101, rtol=0, atol=1e-03)

this np.fromiter is very cool I didn't know about that

JoeZiminski · 2023-03-21T13:59:10Z

tests/test_integration/test_sf_tf.py

+        np.fromiter(p_values.values(), dtype=float), decimal_points
+    )
+    # based on random seed = 101
+    p_values_seed_101 = np.array([0.055, 0.473, 0.324, 0.127, 0.653])


a quick doc on how these test values were generated might be useful. It might be worth considering generating test data and results on the fly as well as having fixed values. The benefit with this is you are not tied to the random seed and generate with random values / arbitary ranges of input data to interrorgate the software a little more and possibly detect edge cases. The downside of this is you have to code the same thing twice - maybe in a different way - but are always liable to make the same error twice. Maybe a mix between hard-coded test values and some randomly-generated inputs is the best of both worlds.

JoeZiminski · 2023-03-21T14:01:13Z

tests/test_integration/test_sf_tf.py

+    sf_tf = get_sf_tf_instance
+    sf_tf.responsiveness()
+
+    assert len(sf_tf.responsive_rois) == 0


overall these tests are very nice and robust!! Is it also feasible to somehow test against their original analysis pipeline? e.g. run their pipeline with some .mat file input data, get their p-values etc. Then make sure all results match using this pipeline? That way you can be 100% sure the implementations yeild identical outputs (or, at least very similar to within some known tolerance).

lauraporta · 2023-03-29T16:11:43Z

Joe, thanks for your comments and suggestions.
Some of them can be relatively quick fixes, other require an extensive refactoring of the classes PhotonData and analysis. To address your concerns, I made single or grouped issues: #37 #38 #39 and #40. Given that the two classes involved in these changes have been affected by PR #27, and also that they are going again through changes once I introduce more of 3p microscopy handling and experiments over multiple days, it makes sense to restructure them only at the very end.
Therefore, I am not going to add further changes on this PR.

JoeZiminski · 2023-03-30T08:27:44Z

Joe, thanks for your comments and suggestions. Some of them can be relatively quick fixes, other require an extensive refactoring of the classes PhotonData and analysis. To address your concerns, I made single or grouped issues: #37 #38 #39 and #40. Given that the two classes involved in these changes have been affected by PR #27, and also that they are going again through changes once I introduce more of 3p microscopy handling and experiments over multiple days, it makes sense to restructure them only at the very end. Therefore, I am not going to add further changes on this PR.

@lauraporta all sounds great, let me know if there's anything you'd like to discuss!

lauraporta added 3 commits February 6, 2023 17:25

Renaming main folder

dea8038

Refactor and remove redundant objects "options" and "specifications"

7b514a5

Add stim matrix logic and flake8 adjustments

92701f5

lauraporta linked an issue Feb 14, 2023 that may be closed by this pull request

Responsiveness method logic #26

Closed

lauraporta added 9 commits February 13, 2023 19:40

Add missing tox dependency

99a2dd7

Improve calculations of response and baseline given a padding

3299b4e

Fix bugs in photon data and add many checks

a28211a

Add tests edits

a31e310

Merge commit '97515c608adbd40860ee8680c07a9f5914edd892' into responsi…

025c5a8

…veness

Add usage of config in phton_data for more controls

4d0fbc2

Update setup_for_demo

6253975

Add logic to calculate mean response and baseline

830958c

Fix test error - silence checks that require real data

b07aab1

lauraporta changed the base branch from developement to reorganize-data February 23, 2023 18:19

lauraporta changed the base branch from reorganize-data to developement February 23, 2023 18:19

lauraporta added 12 commits February 28, 2023 13:57

Add unique stimuli dict and fix start frames bug in photon_data

a6ca434

Change logging and error messages

b6879ef

Add imporvements in windows estimation and initial version of statist…

23cf752

…ical calculations (last one stil; buggy)

Add fixes to anova

a606ac0

Fix signed rank test

8b1d791

Add magintude calculations and refactoring

7294f3d

Add calculations for significance

2ba1527

Reorder methods and add docstrings

a16d127

Refactor and imporve PhotonData, imporve tests

face2f4

Change sf_tf class name

396cf06

Add share pytest.fixture file and sf_tf tests

c04fa15

Add dependencies

904f418

lauraporta marked this pull request as ready for review March 2, 2023 16:04

lauraporta added 2 commits March 2, 2023 16:51

Add data processing description

4634e0d

Add description of SF TF analysis

ed86e70

lauraporta self-assigned this Mar 2, 2023

lauraporta requested a review from JoeZiminski March 2, 2023 17:09

Update setup_for_demo.py

b101958

JoeZiminski reviewed Mar 21, 2023

View reviewed changes

This was referenced Mar 29, 2023

Refactor PhtonData and FrequencyResponsiveness #37

Open

Delete timing column #38

Open

Rethink statistical tests #39

Open

Change black style #40

Closed

lauraporta added this to the First functioning release milestone Mar 29, 2023

JoeZiminski approved these changes Mar 30, 2023

View reviewed changes

lauraporta merged commit 10677fe into developement Mar 30, 2023

lauraporta mentioned this pull request Mar 30, 2023

Plot anova window #30

Closed

lauraporta mentioned this pull request Apr 20, 2023

Improve analysis #49

Merged

lauraporta deleted the responsiveness branch May 9, 2023 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Responsiveness logic #25

Responsiveness logic #25

lauraporta commented Feb 14, 2023

lauraporta commented Mar 2, 2023 •

edited

Loading

JoeZiminski left a comment •

edited

Loading

JoeZiminski Mar 14, 2023

JoeZiminski Mar 14, 2023

lauraporta Mar 29, 2023

JoeZiminski Mar 14, 2023

lauraporta Mar 29, 2023

JoeZiminski Mar 21, 2023

lauraporta Mar 29, 2023

JoeZiminski Mar 21, 2023

lauraporta Mar 29, 2023

JoeZiminski Mar 21, 2023

JoeZiminski Mar 21, 2023

JoeZiminski Mar 21, 2023

lauraporta commented Mar 29, 2023

JoeZiminski commented Mar 30, 2023



		@pytest.fixture
		def get_sf_tf_instance(get_config, get_data_raw_object):

Responsiveness logic #25

Responsiveness logic #25

Conversation

lauraporta commented Feb 14, 2023

lauraporta commented Mar 2, 2023 • edited Loading

What there is in this PR?

JoeZiminski left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lauraporta commented Mar 29, 2023

JoeZiminski commented Mar 30, 2023

lauraporta commented Mar 2, 2023 •

edited

Loading

JoeZiminski left a comment •

edited

Loading