Merged
Conversation
Gives same results as old method :) Must however be invoked differently Todo: compatibility fix
Instead of having this specific reader for reading in whisker-evoked activity data, I simply pre-process the data here. The reader then only needs to worry about fetching the correct file, which I believe is the most configurable and general way it should work. I kept a notebook outlining how exactly I "augmented" the data. This was not very well documented. Added readme to explain the data as well.
Todo: test if this works, gives same results, and can be invoked same way
Had some wrong mappings
…dditional sources of activity
Previously, we recommended users to clone the repo with `depth 1` to save on installation time. The rationale was that older commits were not necessarily useful for the average user. Time has taught us that there are plenty of cases where inspecting older commits, or having access to these, are convenient nonetheless.
Add info on user-wide and system-wide site-packages
Add info on user-wide and system-wide site-packages
…amework into rc-0.7.0-beta
I believe there is some confusion (at least there is on my part) on the intended behavior of renaming `hoc` morphology labels. My intended purpose was to keep the hoc label as it exists in the morphology file, unless explicitly specified otherwise. The only way in which the user was allowed to specify otherwise, is to provide a mapping between labels that may appear in a `hoc` morphology (in full, to avoid confusion), and some desired rename. E.g. the label "BasalDendrite" could be renamed to "Dendrite" by extending the hoc label map with `"basaldendrite": "dendrite"`. It was then up to the user to keep this map unambiguous. The implementation in 208e888 is the other way around. Rather than looking in the hoc map to see if some label (in full) needs to be renamed, it looks through the label, and checks if parts of it match a hoc map key. This was not my intended use case for this hoc label map, as it leads to ambiguous definitions for e.g. `"ApicalDendrite"`, as it will match both `apical` and `dend`. While this error is gracefully handled, it should not produce an error in the first place. Morphology files should be allowed to specify "ApicalDendrite" as a morphology label, and the user should be able to rename labels like "dend" or "apical" to "Dendrite" and "ApicalDendrite" respectively.
`NameError: free variable 'meta' referenced before assignment in enclosing scope` d6ea853 filtered `meta` on its columns when those were defined (great), but `meta` was now left undefined if no columns were defined (not great)
`NameError: free variable 'meta' referenced before assignment in enclosing scope` d6ea853 filtered `meta` on its columns when those were defined (great), but `meta` was now left undefined if no columns were defined (not great)
`hoc` labels are now parsed by considering any suffix that contains underscores and numbers only as the suffix. anything that comes before that is the prefix.
…er" (#493) I believe there is some confusion (at least there is on my part) on the intended behavior of renaming `hoc` morphology labels. My intended purpose was to keep the hoc label as it exists in the morphology file, unless explicitly specified otherwise. The only way in which the user was allowed to specify otherwise, is to provide a mapping between labels that may appear in a `hoc` morphology (in full, to avoid confusion), and some desired rename. E.g. the label "BasalDendrite" could be renamed to "Dendrite" by extending the hoc label map with `"basaldendrite": "dendrite"`. It was then up to the user to keep this map unambiguous. The implementation in 208e888 is the other way around. Rather than looking in the hoc map to see if some label (in full) needs to be renamed, it looks through the label, and checks if parts of it match a hoc map key. This was not my intended use case for this hoc label map, as it leads to ambiguous definitions for e.g. `"ApicalDendrite"`, as it will match both `apical` and `dend`. While this error is gracefully handled, I'd argue this should not produce an error in the first place. Morphology files should be allowed to specify "ApicalDendrite" as a morphology label, and the user should be able to rename labels like "dend" or "apical" to "Dendrite" and "ApicalDendrite" respectively.
This file contains hidden or 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
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.
This is a relatively large refactor to address many reviewer comments, and to make ISF more generally applicable.
swc support
We now support
swc. ISF pipelines are agnostic to whether a morphology file ishocorswc. Reading in and building the morphology is simply handled bysingle_cell_parser.io.morphology.read_morphology. Big thanks to @su-saka and @fkkastrinakis for writing the initial converters, that save me a bunch of time for further integration.We have converters from
swctohocandhoctoswc. Convertinghoctoswccan slightly change connectivity of some sections to the soma, but this is of no impact in ISF as we connect them by default atx=0.5.Simulations on both yield almost identical results. I implemented a test for this. variable time step simulations are not fully identical, fixed timesteps simulations are. I am not sure yet why this is the case. Variable time step simulations for
hocvsswcare virtually indistinguishable, but the time steps are ever so slightly different.#482 and #476
Network mapping generalizibility
I changed the network mapping procedure so it does not depend anymore on any particular label (e.g. "apical"). Instead, labels are inferred from the morphology file. Additional empirical data should then match these labels (e.g. connections spreadsheet) case-insensitively #478 #467
singlecell_input_mapperhas been refactored extensively. It now uses a builder pattern to add network parameters. this makes it way easier to modularize and understand which data is being added, and how (see #465). In addition, the default network embedding strategy can now be configured on a user-level (see #466 )I extended the network parameters with a
NetworkParamBuilderclass that makes it way more convenient to build network parameters, and makes it explicit which data you add, when, and where. This is exploredextensively in the tutorials. #465
Network mapping does not rely anymore on a
NrOfConnections.csv. This used to be default output of our network mapper. However, the same information is contained in a.confile. We now parse it from a.confile. This way, ISF is fully independent of any particular network mapping procedure, and simply relies on asynandconfile.I augmented our dataset for activity data. The correct activity data file + key used to be inferred at runtime, i.e. depending on your recording site and stimulus, it would redirect you to the correct activity data file on the fly. This was very barrel cortex specific. I augmented the activity data by simply generating datasets for all stimuli. The augmentation method is saved as a notebook in
getting_started/functional_constraints/evoked_activity/empirical_recordings. Now, the key you pass under "whisker" is instead interpreted as a globstring that fetches the correct data file in a preconfigured data directory. TheNetworkParamBuilderof course also allows you to directly add activity data, independent of any directory configuration. #465A tutorial now explores all the data requirements for Udvary 2022 #480
The network mapper used in ISF is Udvary 2022, but it should be clear that ISF does not rely on this particular network mapping. I refactored the top-level pipeline so it infers a specific network mapping procedure, and refactored Udvary 2022 as a separate subpackage in
singlecell_input_mapper. #466L5PT mentions
I removed all mentions of "L5PT" in the code, except for those methods that are actually specific to L5PTs. In the latter case, it should be clear that this methods provides an example, and should not live in top-level pipelines or modules #469
Tutorials
Updated the intro tutorial so be more targeted at exactly what ISF does. Removed configuration setup. Added more headings, intro and explanation on what we do, how, and why. #484
Updated network mapping tutorial to add API for the SynapseMapper and visualizations. Added auxiliary tutorials. They are hidden from direct view in the webpage, but reflinks work. This is e.g. used in tutorial 2.1 #485
Config
Added config for morphology labels. You can extend a map so labels identified in morphology files are renamed to something else. This is useful for having a consistent internal representation. I had to do this to rename "BasalDendrite" (in network mapping methods) to "Dendrite"
Added mechanisms to user config, since the mechanisms are user-specific,
technically.
Docs
Added documentation on
swcandmodfiles. Wrote a custom lexer forNMODL :)
Extended installation instructions for more info on how ISF handles environments and env variables.
Extended documentation on file formats, added data sources to paramfile overview
Extended tutorials