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

Save results to a user given output directory #948

Open
wants to merge 43 commits into
base: dev
Choose a base branch
from

Conversation

tdincer
Copy link

@tdincer tdincer commented Feb 5, 2022

Description

The changes made in this PR allows cnmf.fit_file method 1) to save the results in a user given output directory and 2) to return resultant motion correction object, if specified with a flag. All the changes made are backward compatible.

The motivation behind the changes is to incorporate CaImAn into a DataJoint workflow. We realized that the flexibility to save the results into a user given output directory was also thought by the CaImAn developers. As a reference please see the following block of code with comments from the load_memmap function in the mmaping.py file:

if pathlib.Path(filename).suffix != '.mmap':
        logging.error(f"Unknown extension for file {filename}")
        raise ValueError(f'Unknown file extension for file {filename} (should be .mmap)')
    # Strip path components and use CAIMAN_DATA/example_movies
    # TODO: Eventually get the code to save these in a different dir

The changes made in this PR requires only the pathlib python package.

Type of change

  • New feature (non-breaking change which adds functionality)

Has your PR been tested?

This PR has successfully passed both

python caimanmanager.py test

and

python caimanmanager.py demotest

prior to submitting our pull request.

tdincer and others added 7 commits January 18, 2022 15:55
changed `if return_mc:` to ` if return_mc & motion_correct:` to make sure that motion correction is applied if we want to return the `motion correction object`
`base_name` line is changed to its original form.
added user given output directory
@tdincer
Copy link
Author

tdincer commented Mar 2, 2022

Hi @pgunn - I was wondering if you had a chance to look at this PR, and if so, about your opinion.

@pgunn
Copy link
Member

pgunn commented Mar 2, 2022

Hello,
Sorry for not getting back to you on this sooner; I've had a set of changes to Caiman that address this kind of concern (I've been thinking about it for awhile) that are not yet default behaviour. The changes are controlled by two new variables, if you'd like to play around with them (they are not well tested, but may do exactly what you want .. maybe).

Try this:

export CAIMAN_NEW_TEMPFILE=true
export CAIMAN_TEMP=/job/specific/path

(replace that value of CAIMAN_TEMP to something valid on your filesystem)

You'd do this with the version of caiman that does not have your patches; it should achieve the goal of keeping the temporary files of separate jobs distinct from each other.

I've been meaning to make these changes the default (after a lot more extensive testing), but other projects got in the way; I still mean to at some point.

@kushalkolar
Copy link
Collaborator

@pgunn Does setting CAIMAN_TEMP set the mcorr output memmaps to be stored in the CAIMAN_TEMP dir? (I'm looking for a way to get around #950 )

Thanks!

@pgunn
Copy link
Member

pgunn commented Mar 29, 2022

It should (provided CAIMAN_NEW_TEMPFILE is also set to be true)

@kushalkolar
Copy link
Collaborator

kushalkolar commented Apr 17, 2022

I just tried this and it doesn't affect the location of the created memmap for motion correction. From searching the codebase it appears that caiman.paths.fn_relocated (which is the only function I could find that uses CAIMAN_NEW_TEMPFILE), is only used for online motion correction, not the offline motion correction:

https://github.com/flatironinstitute/CaImAn/search?q=fn_relocated%28

@pgunn
Copy link
Member

pgunn commented Apr 18, 2022

Ah, sorry about that. I'll have to improve on this.

@kushalkolar
Copy link
Collaborator

@pgunn no worries, I could try to implement this for offline motion correction and do a PR later this week.

@EricThomson
Copy link
Contributor

Where are we with this PR?

@kushalkolar
Copy link
Collaborator

I did the PR for mcorr so it uses the env var:
#978

For CNMF you can already save the hdf file to whereever you want.

Longterm we should have a better storage system, perhaps in v2. This is part of what mesmerize does :)

Thinh Nguyen and others added 4 commits October 3, 2023 17:32
@pgunn
Copy link
Member

pgunn commented Oct 18, 2023

I'm going to look at this again soon given that you're working on it; given that this is something we've been working on for awhile and we probably want roughly the same end (but not necessarily the same implementation), we'll have to figure out if/how to get this merged in (and whether to sync it with my own efforts or not).

EricThomson and others added 28 commits February 1, 2024 19:51
dev->main for release (bugfix on last release)
Pull changes from `flatironinstitute` and resolve merge conflicts
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.

8 participants