Add Absolute Position Encoder #2

jacobbieker · 2021-10-12T15:35:04Z

Pull Request

Description

Moved from openclimatefix/nowcasting_utils#33

Adds positional encoders for:

Absolute Position Encodings

It is generated using Fourier Features for now, although other options could include coordinates, such as CoordConv or others.

Fixes #4

How Has This Been Tested?

Unit Tests, which pass for the encoding. Some fail for the dataloader/datamodule, but that's unrelated to this

No
Yes

Checklist:

My code follows OCF's coding style guidelines
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked my code and corrected any misspellings

flowirtz

Few smaller comments, but after fixing that LGTM! 👍

Some [tests] fail for the dataloader/datamodule

Are we tracking those anywhere already?

flowirtz · 2021-10-14T14:45:51Z

nowcasting_dataloader/utils/position_encoding.py

@@ -153,7 +139,7 @@ def encode_absolute_position(


 def normalize_geospatial_coordinates(
-    geospatial_coordinates, geospatial_bounds: Dict[str, float], **kwargs
+    geospatial_coordinates: List[np.ndarray], geospatial_bounds: Dict[str, float], **kwargs


yay, more types!

nowcasting_dataloader/utils/position_encoding.py

tests/test_position_encoding.py

Co-authored-by: Flo <[email protected]>

jacobbieker · 2021-10-14T16:12:37Z

Few smaller comments, but after fixing that LGTM! +1

Some [tests] fail for the dataloader/datamodule

Are we tracking those anywhere already?

No, not right now, it seems like some changes in nowcasting-dataset meant the dataloading code I copied from SatFlow fails. Since nowcasting-dataset is still changing quite a bit with openclimatefix/nowcasting_dataset#213 I was going with sorta waiting till that's more finished to fix the tests. I'll make pytest skip them actually for now

jacobbieker · 2021-10-14T16:26:42Z

Few smaller comments, but after fixing that LGTM! +1

Some [tests] fail for the dataloader/datamodule

Are we tracking those anywhere already?

No, not right now, it seems like some changes in nowcasting-dataset meant the dataloading code I copied from SatFlow fails. Since nowcasting-dataset is still changing quite a bit with openclimatefix/nowcasting_dataset#213 I was going with sorta waiting till that's more finished to fix the tests. I'll make pytest skip them actually for now

I'll be making these tests pass as part of #7

JackKelly

Looks really great to me! Very nicely structured code - easy to read!

A few comments, mostly pretty small!

nowcasting_dataloader/utils/position_encoding.py

JackKelly · 2021-10-20T10:33:27Z

nowcasting_dataloader/utils/position_encoding.py

+
+
+def encode_modalities(
+    modalities_to_encode: Dict[str, torch.Tensor],


I wonder if we should use an Enum for modality names, instead of strings? I'll start a new issue: #15

nowcasting_dataloader/utils/position_encoding.py

Co-authored-by: Jack Kelly <[email protected]>

…cob/position-encoding

Co-authored-by: Jack Kelly <[email protected]>

…cob/position-encoding

jacobbieker added the enhancement New feature or request label Oct 12, 2021

jacobbieker self-assigned this Oct 12, 2021

Move position_encoding from nowcasting_dataset PR

7b677d5

jacobbieker force-pushed the jacob/position-encoding branch from 50d3c36 to 7b677d5 Compare October 13, 2021 09:00

jacobbieker added 6 commits October 13, 2021 10:51

Start on encoding modalities checks

4337828

Add getting needed information for relative encoding

46b7f5f

Add absolute encoding, more steps for relative encoding

e81cc10

Make stub for subselecting position encoding

990b22b

Add first bit of subselecting position encoding tensor

53b34e8

Remove relative encoding, split into different branch

62eb6d9

jacobbieker changed the title ~~Add Positional Encoders~~ Add Absolute Position Encoder Oct 13, 2021

This was referenced Oct 13, 2021

"Big new design" for nowcasting_dataset openclimatefix/nowcasting_dataset#213

Closed

Add Joint Model openclimatefix/satflow#102

Closed

jacobbieker added 13 commits October 13, 2021 16:07

Go through the batch of datetimes

04e8de5

Simplify code

7da54df

Add datetime feature creation test

0d27504

Add test for normalizing geospatial coordinates

f635872

Add test for absolute position encoding

d88f826

Run black on tests

01246ad

Add test for encode position

33791a7

Add not implemented test

bcf6a5d

Add test for encoding modalities

fe01d2f

Add test for multi-modality encoding

c283baa

Add PV to multi-modality test

0d6612b

Remove stub tests

61a5347

Add requirement

79b9e21

jacobbieker marked this pull request as ready for review October 14, 2021 07:55

jacobbieker requested review from flowirtz and JackKelly October 14, 2021 07:55

Remove relative and both options for encoding

0a5ba2e

flowirtz approved these changes Oct 14, 2021

View reviewed changes

jacobbieker and others added 2 commits October 14, 2021 17:09

Update nowcasting_dataloader/utils/position_encoding.py

0f45431

Co-authored-by: Flo <[email protected]>

Update tests/test_position_encoding.py

9f40ac8

Co-authored-by: Flo <[email protected]>

Add skipping tests

ba7b484

jacobbieker mentioned this pull request Oct 14, 2021

Add Absolute Positional Encoding in Dataloader #7

Merged

7 tasks

Merge branch 'main' into jacob/position-encoding

33c2903

JackKelly approved these changes Oct 20, 2021

View reviewed changes

jacobbieker and others added 16 commits October 20, 2021 12:53

Update nowcasting_dataloader/utils/position_encoding.py

f1c1f6e

Co-authored-by: Jack Kelly <[email protected]>

Update nowcasting_dataloader/utils/position_encoding.py

950f7cd

Co-authored-by: Jack Kelly <[email protected]>

Update nowcasting_dataloader/utils/position_encoding.py

6bf2f3b

Co-authored-by: Jack Kelly <[email protected]>

Update nowcasting_dataloader/utils/position_encoding.py

d14ee1a

Co-authored-by: Jack Kelly <[email protected]>

Address some PR comments

fcf551e

Merge remote-tracking branch 'origin/jacob/position-encoding' into ja…

b04cdd5

…cob/position-encoding

Update nowcasting_dataloader/utils/position_encoding.py

1de189f

Co-authored-by: Jack Kelly <[email protected]>

Address more PR comments

2f4319f

Merge remote-tracking branch 'origin/jacob/position-encoding' into ja…

5da3588

…cob/position-encoding

Remove encode_position

f7b46b9

Switch to using constants

6e8036b

Remove method

ac10f76

Fix import

197bc93

Fixes

6e273b5

Fix test

0326f76

Change docstring

ce9a595

jacobbieker merged commit 2de25e2 into main Oct 20, 2021

jacobbieker deleted the jacob/position-encoding branch October 20, 2021 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Absolute Position Encoder #2

Add Absolute Position Encoder #2

jacobbieker commented Oct 12, 2021 •

edited

Loading

flowirtz left a comment

flowirtz Oct 14, 2021

jacobbieker commented Oct 14, 2021

jacobbieker commented Oct 14, 2021

JackKelly left a comment

JackKelly Oct 20, 2021



		def encode_modalities(
		modalities_to_encode: Dict[str, torch.Tensor],

Add Absolute Position Encoder #2

Add Absolute Position Encoder #2

Conversation

jacobbieker commented Oct 12, 2021 • edited Loading

Pull Request

Description

How Has This Been Tested?

Checklist:

flowirtz left a comment

Choose a reason for hiding this comment

flowirtz Oct 14, 2021

Choose a reason for hiding this comment

jacobbieker commented Oct 14, 2021

jacobbieker commented Oct 14, 2021

JackKelly left a comment

Choose a reason for hiding this comment

JackKelly Oct 20, 2021

Choose a reason for hiding this comment

jacobbieker commented Oct 12, 2021 •

edited

Loading