Split ROMSComponent.input_datasets attribute up
#95
Conversation
This file contains 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
added 3 commits
September 18, 2024 17:30
- Call these methods from inside `Case.from_blueprint` and `Case.persist` - Remove reference to `input_datasets` in Component base class
- Add attributes model_grid, initial_conditions, surface_forcing, tidal_forcing, boundary_forcing on ROMSComponent - Define from_dict on ROMSComponent calling super().from_dict before adding roms-specific attributes Necessary refactoring: - Redefine datasets_to_partition list in ROMSComponent.pre_run using these attributes - Remove 'discretization' attr from base Component class (as not needed by MARBL) - Move Discretization and ROMSDiscretization classes to their own modules - Add __init__ and from_dict to MARBLComponent - Remove "name" entry from blueprint under "base_model": 'name' is not a constructor arg to BaseModel class... - ... Define "name" entry in blueprint at the component level instead
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.
The key goal here is to split
ROMSComponent.input_datasetsup into several more specific attributes, e.g.ROMSComponent.model_grid, closing #94 . This has implications in other places in the code.It's already clear this is leading to a lot of reshuffling. I pondered for a good while yesterday how to split the various obstacles up, but they're pretty entangled, so I'm going to open this as a draft and push (non-working) commits to hopefully make it easier to follow along.
Problem 1
Description
Case.from_blueprintcreates all the objects that go into a component, including InputDataset objects, before creating the Component object itself from those objects. As such it already has some nasty logic that is hyper-specific to, e.g. ROMSComponent. Implementing this change toROMSComponent.input_datasetswill involve rewriting large parts ofCase.from_blueprint, and it would make sense to hand off component-specific aspects like this to the relevant Component subclasses.Attempted solution
To start, I'm attempting to create a
Component.from_dictclass method, so thatCase.from_blueprintcan just call this method on each component in turn, passing the relevant subsection of the blueprint yaml file.