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.

Sign up for GitHub

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

Jump to bottom

Multisession modeling functionality for AutoLFADS #152

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Multisession modeling functionality for AutoLFADS #152

wants to merge 27 commits into from

Conversation

claytonwashington
Copy link

No description provided.

@claytonwashington
attempt 1 at full changes for msm
fb2e64b
@claytonwashington
stash for testing
fe77fab
@claytonwashington
post-testing changes
2e50d16
@claytonwashington
test
f92f4ff
@claytonwashington
fix
7415356
@claytonwashington
automatic commit to document pipeline stack_config_template.json befo…
f185ba7 
…re update at 2023-10-03 16:52:48.997520
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
0cb782e 
…r update at 2023-10-03 16:52:49.268505. Purpose: test. AMI: ami-05ddab33dd696e7d0
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
4ea3dec 
…r update at 2023-10-04 11:00:56.540733. Purpose: setup before first single run. AMI: ami-076b9ecc29502b344
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
7700632 
…r update at 2023-10-09 17:07:12.925453. Purpose: upgraded CUDA. AMI: ami-0295578a40774d483
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
6f3c55d 
…r update at 2023-10-09 17:40:31.179587. Purpose: updated cuda and got sklearn. AMI: ami-0163b17cdfa51a9f5
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
33edce2 
…r update at 2023-10-10 17:08:18.822096. Purpose: final test on local single and multi. AMI: ami-0ccfff108ae8de464
@cellistigs
Copy link
Collaborator

Confirmed unit tests passed on local machine. Moving on to tests with AWS SAM.

@cellistigs
Copy link
Collaborator

cellistigs commented Oct 27, 2023 •
edited
Loading

Sam package built/Template validated.

$ cd ~/neurocaas/ncap_iac/ncap_blueprints/iac_utils/
$ bash fullbuild.sh ../autolfads-torch
$ sam validate --template-file

@cellistigs
Copy link
Collaborator

cellistigs commented Oct 27, 2023 •
edited
Loading

Built AWS sam package successfully (docker failing before- updated fullbuild.sh to use appropriate context.)

@cellistigs
Copy link
Collaborator

Need multisession parameter for this analysis to pass in config file.

@cellistigs
Copy link
Collaborator

cellistigs commented Oct 27, 2023 •
edited
Loading

Tested standard implementation. Across various errors (different environment variables, misspecified arns) preserved behavior of shutting down instances as expected.

@cellistigs
Copy link
Collaborator

At the moment, ncap_iac/protocols/submit_start.py has both process_upload_dev and process_upload_multisession. It appears these are identical save for the submission class passed to them. It would be good to refactor this so we can pass a submission object- since these functions take care of error handling having a single function wherever possible will simplify testing in the future.

@cellistigs
Copy link
Collaborator

The following functionality has been tested for reliability of cloud management systems, validated through sam local invoke calls. Note that we only check MainLambda here, as the behavior of the postprocessing lambda file does not have to change.

  • failing to pass a multisession parameter
  • passing multisession:False with a single dataset in the inputs folder.
  • passing multisession:True with a single dataset in the inputs folder.
  • passing multisession:True with datasets in multisession_test (directory passed to submit file) (launches one instance)
  • passing multisession:False with datasets in multisession_test (directory passed to submit file) (launches one instance)
  • passing multisession:True with datasets in multisession_test (individual files passed to submit file) (launches two instances)
  • passing multisession:False with datasets in multisession_test (individual files passed to submit file) (launches two instance)

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

We have some issues with logging- at least on AWS-SAM, we only ever see messages from logging start of the first instance. Check this in deployment. Note we have only checked the above functions for cloud reliability, so the above formats should be tested for expected functionality in all combinations of inputs listed above.

We should also refactor: At the moment, ncap_iac/protocols/submit_start.py has both process_upload_dev and process_upload_multisession. It appears these are identical save for the submission class passed to them. It would be good to refactor this so we can pass a submission object- since these functions take care of error handling having a single function wherever possible will simplify testing in the future.

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

1 similar comment
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@claytonwashington
no multisession field in config defaults to false
b031c66
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
025c8f8 
…r update at 2023-10-31 14:18:09.494446. Purpose: multisession entry no longer needed in config. AMI: ami-0699e307bc81482db
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
39badb9 
…r update at 2023-11-10 15:49:37.826420. Purpose: fixed lfads-torch import. AMI: ami-0893ac69ba200a546
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
6400428 
…r update at 2023-11-16 13:55:30.184299. Purpose: fixed issues with access for root/SSM user. AMI: ami-017465330c3757649
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
1540f23 
…r update at 2023-11-16 14:25:10.865461. Purpose: root path fix fix?. AMI: ami-0754772739da3a57d
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@claytonwashington
drive run_main.py with run_main.sh
3207d97
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
51303b6 
…r update at 2023-11-28 17:12:13.330582. Purpose: fixed single dataset run; command broken for multisession. AMI: ami-024a73c9849ee99e4
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
a02e41d 
…r update at 2023-12-01 21:09:51.895815. Purpose: new multisession post-tutorial. AMI: ami-0825772e0ac2e67c8
@claytonwashington
automatic commit to document pipeline stack_config_template.json afte…
1b2d533 
…r update at 2023-12-05 22:16:10.678782. Purpose: successful ssm test on multisession via nc_contrib hotfix 51. AMI: ami-0cebcfea203997fcf
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

cellistigs
cellistigs reviewed Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@cellistigs cellistigs left a comment

Choose a reason for hiding this comment

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

Most of the changes here look good- however I have some suggestions regarding code structure and logging.

ncap_iac/protocols/submit_start.py Outdated Show resolved Hide resolved
ncap_iac/protocols/submit_start.py Outdated Show resolved Hide resolved
ncap_iac/protocols/submit_start.py Outdated Show resolved Hide resolved
ncap_iac/protocols/submit_start.py Show resolved Hide resolved
@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

@cellistigs
Copy link
Collaborator

#deploy:autolfads-torch

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

@cellistigs cellistigs cellistigs left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@claytonwashington @cellistigs