0.1.0 release #226
0.1.0 release #226
Conversation
| source | ||
| ------ | ||
|
|
||
| TorchGeo can also be installed from source using the ``setup.py`` file and setuptools. |
There was a problem hiding this comment.
Direct invocation of setup.py is deprecated: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html
| "program.log_dir=" + str(log_dir), | ||
| "trainer.fast_dev_run=1", | ||
| "experiment.task=" + task, | ||
| "program.overwrite=True", | ||
| "config_file=" + os.path.join("conf", "task_defaults", task + ".yaml"), |
There was a problem hiding this comment.
@calebrob6 without this line, the default configs aren't being loaded, so there might be something broken with default loading.
There was a problem hiding this comment.
The default config is loaded by name here -- https://github.com/microsoft/torchgeo/blob/main/train.py#L97.
There was a problem hiding this comment.
Yes, but if you comment out this line, you'll see that it isn't loading the default config.
There was a problem hiding this comment.
It definitely is loading the default config, else it would throw an error about not finding the default config
There was a problem hiding this comment.
Because of the way configurations are merged, experiment.datamodule.root_dir was being overridden by the defaults in defaults.yaml (i.e. program.data_dir). I'm not sure it makes sense to have the test datasets be the default data for each dataset. The fixes here are:
- do as you've done
- remove the default mapping between
program.data_dirandexperiment.datamodule.root_dirindefaults.yaml - remove the default root_dirs that you added to all the datasets and pass those instead as arguments to
program.data_dir - rewrite the configuration system :)
There was a problem hiding this comment.
Yeah, I realize I've been abusing the config system by modifying it to suit the tests. What if we:
- Keep doing this until we get the release out since the
confdirectory won't get installed anyway - After the release is out, create a
tests/confdirectory for test-specific configuration settings - Think about Re-think how configs are handled in train.py #227 post-rebuttal when we have more time
We could also do 2 real quick, it wouldn't take that long.
There was a problem hiding this comment.
Can you make a TODO here or somewhere that we'll remember?
There was a problem hiding this comment.
I'll create an issue so we don't re-trigger the tests
| - name: Install pip dependencies | ||
| run: pip install .[tests] | ||
| run: | | ||
| pip install gdal tqdm # TODO: these deps shouldn't be needed |
There was a problem hiding this comment.
@isaaccorley the indices tutorial imports things that aren't TorchGeo deps, does it need to?
There was a problem hiding this comment.
I can revamp them but requiring that notebooks only use torchgeo deps might be too restrictive. E.g we may want to use visualization libraries in a tutorial but not add them as a dependency to torchgeo.
There was a problem hiding this comment.
The code definitely uses these two libraries.
- tqdm is simple to include as a dependency, it will be indirectly included anyway, and is quite useful in notebooks and command line scripts
- The gdal bit can be replaced quite easily with rasterio
There was a problem hiding this comment.
Agreed, let's do this for the next release
| - name: Run notebook checks | ||
| env: | ||
| MLHUB_API_KEY: ${{ secrets.MLHUB_API_KEY }} |
There was a problem hiding this comment.
Alternatively, we could use a different dataset that doesn't require an API key to download
There was a problem hiding this comment.
I'd rather not have an API key secret, and would rather use UCMerced instead of Cyclone. Lets put that on the 0.2 release path.
| @@ -451,3 +451,6 @@ def validation_step( # type: ignore[override] | |||
| ) | |||
|
|
|||
| self.log("val_loss", loss, on_step=False, on_epoch=True) | |||
|
|
|||
| def test_step(self, *args: Any) -> None: # type: ignore[override] | |||
| """No-op, does nothing.""" | |||
There was a problem hiding this comment.
PyTorch Lightning 1.5 added a feature to check the validity of a trainer. If you pass a DataModule with a test_loader but your trainer doesn't have a test_step, it raises an error.
* 0.1.0 release * Train deps needed for release testing * Update development status * setup.py should not be run directly * Test more trainers * Fix local docs build * Update installation instructions * Specify test data dir in config * Fix tutorial docs * Trainers should default to num_workers=0, download=False * Correct location for root_dir * Try different GDAL name * Try again * Various fixes to release tests * Update pip installs in tutorials * Fix some bugs * Config file not being picked up * Get back to 100% test coverage * Added correct weight string to UCMerced * yolo fix * yolo fix pt 2 * yolo fix 2 pt. 1 * Simplify tests a bit * Make the trainer notebook look stupid * UCMerced should download by default in the trainers * Revert * Fix logo/author, include LICENSE in upload Co-authored-by: Caleb Robinson <[email protected]>
See https://github.com/microsoft/torchgeo/wiki/Releasing-Instructions
Closes #55