initial cli files, core and solvents #9
Conversation
| # Confirm creation | ||
| if not typer.confirm("Create project with these settings?"): | ||
| raise typer.Abort() |
There was a problem hiding this comment.
This might be an issue if we want to use the app in a pipeline.
| # Confirm creation | |
| if not typer.confirm("Create project with these settings?"): | |
| raise typer.Abort() |
|
|
||
| except Exception as e: | ||
| DisplayUtils.show_error(f"Project creation failed: {e}") | ||
| raise typer.Exit(1) |
There was a problem hiding this comment.
i'm not 100% sure about this. maybe at least we could do
| raise typer.Exit(1) | |
| raise typer.Exit(1) from e |
but i'd need to take a look
| # Show summary | ||
| self._show_project_summary(request) | ||
|
|
||
| # Confirm | ||
| if not typer.confirm("Create project?"): | ||
| raise typer.Abort() | ||
|
|
||
| # Create project | ||
| with console.status("[bold green]Creating project..."): | ||
| response = self.service.setup_project(request) | ||
|
|
||
| self._show_creation_results(response) |
There was a problem hiding this comment.
This already happened in the previous command.
It feels like it should be a function taking a ProjectRequest object as a parameter
| def _show_project_summary(self, request: ProjectRequest): | ||
| """Display project configuration summary""" | ||
|
|
||
| summary_table = DisplayUtils.create_table("Project Configuration", | ||
| ["Setting", "Value"]) | ||
|
|
||
| summary_table.add_row("Project Name", request.name) | ||
| summary_table.add_row("System", request.system.name) | ||
| summary_table.add_row("Structure", str(request.system.structure_file)) | ||
| summary_table.add_row("Total Replicas", str(request.replicas.count)) | ||
| summary_table.add_row("Solvents", ", ".join(set(request.replicas.solvents))) | ||
| summary_table.add_row("Simulation Time", f"{request.simulation.total_time} ns") | ||
| summary_table.add_row("Temperature", f"{request.simulation.temperature} K") | ||
| summary_table.add_row("Ensemble", request.simulation.ensemble.value) | ||
| summary_table.add_row("Restraints", request.simulation.restraint_mode.value) | ||
|
|
||
| console.print(summary_table) |
There was a problem hiding this comment.
This could be a free function, probably, as it doesn't depend directly on the cli plugin
| def _show_creation_results(self, response): | ||
| """Display project creation results""" | ||
|
|
||
| if response.status == "success": | ||
| DisplayUtils.show_success(f"Project created successfully!") | ||
| else: | ||
| DisplayUtils.show_warning(f"Project created with issues") | ||
|
|
||
| # Summary | ||
| console.print(f"\nProject Directory: [bold]{response.project_dir}[/bold]") | ||
| console.print(f"Total Replicas: {response.total_replicas}") | ||
| console.print(f"Successful: [green]{response.successful_replicas}[/green]") | ||
|
|
||
| if response.successful_replicas < response.total_replicas: | ||
| failed = response.total_replicas - response.successful_replicas | ||
| console.print(f"Failed: [red]{failed}[/red]") | ||
|
|
||
| # Show any errors | ||
| if response.system.errors: | ||
| DisplayUtils.show_error("System setup errors:") | ||
| for error in response.system.errors: | ||
| console.print(f" - {error}") | ||
|
|
||
| # Show failed replicas | ||
| failed_replicas = [r for r in response.replicas if r.status == "failed"] | ||
| if failed_replicas: | ||
| DisplayUtils.show_error("Failed replicas:") | ||
| for replica in failed_replicas: | ||
| console.print(f" - {replica.name}: {replica.error}") | ||
|
|
||
| # Next steps | ||
| console.print("\n[bold]Next Steps:[/bold]") | ||
| console.print("1. Change to project directory:") | ||
| console.print(f" cd {response.project_dir}") | ||
| console.print("2. Run system preparation:") | ||
| console.print(" ./scripts/setup_project.sh") | ||
| console.print("3. Submit simulations to queue or run locally") No newline at end of file |
There was a problem hiding this comment.
Same thing.
if something, both functions would belong with the 'back-end' side
the cli should probably be just a dumb front-end.
if we're following hexagonal architecture / ddd, this would probably be at the service level
| name: str = Field(..., description="System name") | ||
| structure_file: Path = Field(..., description="Path to OFF or PDB file") | ||
| structure_type: str = Field("off", description="Structure file type (off/pdb)") | ||
| force_fields: List[str] = Field( |
There was a problem hiding this comment.
since some time ago, we don't need to import typing.List anymore,
we can go with just list (same thing with many other collections and types)
| force_fields: List[str] = Field( | |
| force_fields: list[str] = Field( |
| """Request model for system setup""" | ||
| name: str = Field(..., description="System name") | ||
| structure_file: Path = Field(..., description="Path to OFF or PDB file") | ||
| structure_type: str = Field("off", description="Structure file type (off/pdb)") |
There was a problem hiding this comment.
we could do with a str enum, but it might be overkill
another way would be to use the union of Literals
| structure_type: str = Field("off", description="Structure file type (off/pdb)") | |
| structure_type: Literal["off", "pdb"] = Field("off", description="Structure file type (off/pdb)") |
| @field_validator('structure_file') | ||
| @classmethod | ||
| def validate_structure_file(cls, v): | ||
| if not v.exists(): | ||
| raise ValueError(f"Structure file not found: {v}") | ||
| return v |
There was a problem hiding this comment.
this validator is going to be a bit of a pain if we want to create the templates from the pydantic models, let's either comment it out, or use it not as a field_validator but as some sort of model function later.
| force_fields=force_fields | ||
| ), | ||
| replicas=ReplicaRequest( | ||
| count=len(solvents) * replicas, |
There was a problem hiding this comment.
| count=len(solvents) * replicas, | |
| count=replicas, |
| @field_validator('solvents') | ||
| @classmethod | ||
| def validate_solvents(cls, v, info): # Change parameter name | ||
| # Access other field values through info.data | ||
| if info.data and 'count' in info.data: | ||
| count = info.data['count'] | ||
| if len(v) != count: | ||
| # If single solvent provided, replicate for all replicas | ||
| if len(v) == 1: | ||
| return v * count | ||
| raise ValueError(f"Number of solvents ({len(v)}) must match replica count ({count})") | ||
| return v |
There was a problem hiding this comment.
I don't think this is right (?)
we could have just a single solvent with 3 replicas, and the validator would fail that (?)
| class ReplicaRequest(BaseModel): | ||
| """Request model for replica configuration""" | ||
| count: int = Field(3, ge=1, description="Number of replicas") | ||
| solvents: List[str] = Field(..., description="List of solvents for replicas") | ||
| naming_scheme: str = Field("{solvent}_{index}", description="Replica naming pattern") |
There was a problem hiding this comment.
I think it could be interesting to have something like this:
| class ReplicaRequest(BaseModel): | |
| """Request model for replica configuration""" | |
| count: int = Field(3, ge=1, description="Number of replicas") | |
| solvents: List[str] = Field(..., description="List of solvents for replicas") | |
| naming_scheme: str = Field("{solvent}_{index}", description="Replica naming pattern") | |
| class ReplicaRequest(BaseModel): | |
| """Request model for replica configuration""" | |
| solvents: dict[str, int] = Field(default_factory=lambda: {"WAT": 3}, description="dictionary of solvent name to number of replicas") |
maybe the default should be just an empty dict, but this is okay
| class ProjectRequest(BaseModel): | ||
| """Complete project request model""" | ||
| name: str = Field(..., description="Project name") | ||
| description: str = Field("", description="Project description") | ||
| system: SystemRequest | ||
| replicas: ReplicaRequest | ||
| simulation: SimulationRequest | ||
| output_dir: Optional[Path] = Field(None, description="Output directory") | ||
| queue_system: QueueSystem = Field(QueueSystem.LOCAL, description="Queue system") |
There was a problem hiding this comment.
it will be better for us if we make the structure of project request to match the yaml config, as it will make it trivial to both parse the config and generate the template
| } | ||
|
|
||
|
|
||
| class SolventInfo(BaseModel): |
There was a problem hiding this comment.
just for consistency
| class SolventInfo(BaseModel): | |
| class SolventInfoResponse(BaseModel): |
| """Save configuration to YAML file""" | ||
| import yaml | ||
| with open(path, 'w') as f: | ||
| yaml.dump(self.dict(), f, default_flow_style=False) |
There was a problem hiding this comment.
i think the current way of doing that would be
| yaml.dump(self.dict(), f, default_flow_style=False) | |
| yaml.dump(self.model_dump(), f, default_flow_style=False) |
Also, it is possible that we want to use pydantic-settings BaseSettings
https://docs.pydantic.dev/latest/concepts/pydantic_settings/
| @classmethod | ||
| def from_yaml(cls, path: Path): | ||
| """Load configuration from YAML file""" | ||
| import yaml | ||
| with open(path, 'r') as f: | ||
| data = yaml.safe_load(f) | ||
| return cls(**data) |
There was a problem hiding this comment.
This can be done directly with the pydantic
No description provided.