Skip to content

Fix for run_cluster using snakemake.yaml with nwb_template#895

Merged
milesAraya merged 6 commits intodevelop-mainfrom
fix/run_cluster
Aug 7, 2025
Merged

Fix for run_cluster using snakemake.yaml with nwb_template#895
milesAraya merged 6 commits intodevelop-mainfrom
fix/run_cluster

Conversation

@milesAraya
Copy link
Copy Markdown
Collaborator

@milesAraya milesAraya commented Aug 1, 2025

Update to correctly read snakemake.yaml nwb_template format with run_cluster

Content

Recently snakemake.yaml were updated to use a nwb_template instead of repeating the same template many times in the yaml file. However, when combined with run_cluster this caused path issues.

Main Issue:

Circular Dependency in NWB Reference Resolution

  1. NWB Template References: Some snakemake.yaml files use nwbfile: ref:nwb_template instead of embedding
    the full NWB data inline
  2. Reference Resolution: During Snakemake execution, resolve_nwbfile_reference() tried to resolve these
    references by re-reading the config file
  3. Circular Path Logic: The function used SmkConfigReader.read_from_path() which:
    - Extracted workspace_id/unique_id from the provided path
    - Reconstructed a new path using get_config_yaml_path()
    - Called ConfigReader.read() which returned {} (empty dict) due to path issues
    - Failed assertion: assert config, f"Invalid config yaml file..."

Fix:

File: studio/app/common/core/snakemake/smk_utils.py

  • Modified resolve_nwbfile_reference() to accept config parameter from Snakemake context
  • Direct Config Usage: Instead of re-reading files, use snakemake.config passed from rule execution
  • Fallback Logic: Maintained backwards compatibility with file reading for other use cases
  • Updated Call Sites: Modified func.py and data.py to pass snakemake.config
Before: Circular file reading

config = SmkConfigReader.read_from_path(config_path) # Failed with {}

After: Direct config usage

rule_config = SmkUtils.resolve_nwbfile_reference(rule_config, snakemake.config)

Secondary Issue: Windows Path Handling

  • Windows used user temp directories (C:\Users...\AppData\Local\Temp) instead of system temp (C:\temp) for saving, even though the app and the input data was saved correctly in (C:\temp). It seems best to keep Windows temp dir fixed at C:\temp because otherwise, the output of run_cluster is saved to \AppData\Local\Temp and then deleted on cleanup
  • Results saved to wrong location and got cleaned up

Fix:

File: studio/app/dir_path.py

  • Consistent Defaults: Windows now defaults to C:\temp\studio instead of user temp directory
  • Proper Path Separators: Use os.path.join() instead of hardcoded / separators
  • Cross-Platform Consistency: Both Mac (/tmp/studio) and Windows (C:\temp\studio) use system-wide
    locations

References

Link to nwb_template PR
Link to run_cluster PR


Testcase

Comment thread run_cluster.py Outdated
Comment thread studio/app/common/core/snakemake/smk_utils.py
Comment thread studio/app/common/core/snakemake/smk_utils.py
Comment thread run_cluster.py Outdated
Comment thread studio/app/dir_path.py
Comment thread studio/app/common/core/snakemake/smk_utils.py
Copy link
Copy Markdown
Collaborator

@itutu-tienday itutu-tienday left a comment

Choose a reason for hiding this comment

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

I have confirmed that it works, so I approve it.

  • platform: Mac
  • test cases
    • snakemake_nwb_template.yaml.txt ... NG (snakemake execution failed.)

      I'll approve it, but this case still shows an error (there's almost no error log).
      If there seems to be a problem, please check it.

    • snakemake_no_template.yaml.txt ... OK
    • snakemake_error_sample.yaml.txt ... OK

Also, a Milestone (release version) has not been specified for this PR. Please specify one.

@milesAraya milesAraya added this to the v2.4.0 milestone Aug 5, 2025
@milesAraya milesAraya requested review from itutu-tienday and removed request for tsuchiyama-araya August 6, 2025 02:22
@milesAraya milesAraya merged commit fe65c3b into develop-main Aug 7, 2025
5 checks passed
@itutu-tienday itutu-tienday added the bug Something isn't working label Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants