Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More linting. Ignore ESP file. Clean redundancy in CI. #78

Merged
merged 4 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/eluc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
pip install pylint
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
- name: Lint with PyLint
run: pylint --ignore="demo" --recursive=y --fail-under=9 ./*
run: pylint ./*
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't need these flags because they're specified in pylintrc now

- name: Run unit tests
run: python -m unittest

8 changes: 6 additions & 2 deletions use_cases/eluc/.pylintrc
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
[MASTER]
ignore=demo
ignore=demo, prescriptors/esp

recursive=y

fail-under=9.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved flags from eluc.yml into here. Now we ignore the esp directory because it can't be installed in the Github environment

Copy link
Member

Choose a reason for hiding this comment

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

Your code has been rated at 9.38/10

Next, we should raise the bar to 10 and fix the warnings.


jobs=0

max-line-length=120

suggestion-mode=yes

good-names=X_train, X_val, X_test, y_train, y_val, y_test, X, Y, y, X_test_scaled
good-names=X_train, X_val, X_test, y_train, y_val, y_test, X, Y, y, X_test_scaled, df, da, ds, i, n
2 changes: 1 addition & 1 deletion use_cases/eluc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ BLUE simulations with committed emissions could be used to estimate the long-ter
"Committed emissions" means all the emissions that are caused by a land-use change event are attributed to the year
of the event.
BLUE (bookkeeping of land use emissions) is a bookkeeping model that attributes carbon fluxes to land use activities.
See [BLUE: Bookkeeping of land use emissions](https://doi.org/10.1002/2014GB004997) for more details.
See [BLUE: Bookkeeping of Land Use Emissions](https://doi.org/10.1002/2014GB004997) for more details.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tried and failed to fix a weird error: README.md:26:74: E0001: Parsing failed: 'invalid decimal literal (, line 26)' (syntax-error)

Copy link
Member

Choose a reason for hiding this comment

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

Not sure why pylint evaluating README.md. We can probably ignore these files with:

ignore-patterns=*.md

That can investigate when we fix the warnings


### LUC

Expand Down
4 changes: 3 additions & 1 deletion use_cases/eluc/data/conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ def construct_countries_df():
for i in range(len(countries_df)):
old_abbrev = countries_df.iloc[i]["abbrevs"]
if old_abbrev in MANUAL_MAP and MANUAL_MAP[old_abbrev] in codes_df["Numeric code"].unique():
countries_df.iloc[i]["abbrevs"] = codes_df[codes_df["Numeric code"] == MANUAL_MAP[old_abbrev]]["Alpha-2 code"].iloc[0]
numeric_code = codes_df["Numeric code"]
old_abbrev = MANUAL_MAP[old_abbrev]
countries_df.iloc[i]["abbrevs"] = codes_df[numeric_code == old_abbrev]["Alpha-2 code"].iloc[0]

return countries_df
2 changes: 1 addition & 1 deletion use_cases/eluc/prescriptors/nsga2/candidate.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, in_size: int, hidden_size: int, out_size: int,
self.parents = parents

@classmethod
def from_crossover(cls, parent1, parent2, p_mutation: float, gen: int, cand_id: int):
def from_crossover(cls, parent1, parent2, p_mutation: float, gen: int, cand_id: int) -> "Candidate":
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

class method returns itself

"""
Crossover two parents to create a child.
Take a random 50/50 choice of either parent's weights
Expand Down
2 changes: 1 addition & 1 deletion use_cases/eluc/prescriptors/nsga2/nsga2_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def calculate_crowding_distance(front):
n_objectives = len(front[0].metrics)
distances = [0 for _ in range(len(front))]
for m in range(n_objectives):
front.sort(key=lambda c: c.metrics[m])
front.sort(key=lambda candidate: candidate.metrics[m])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

c to candidate

obj_min = front[0].metrics[m]
obj_max = front[-1].metrics[m]
distances[0] = float('inf')
Expand Down
16 changes: 8 additions & 8 deletions use_cases/eluc/prescriptors/nsga2/torch_prescriptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def __init__(self,
batch_size: int,
candidate_params: dict,
seed_dir=None):

self.candidate_params = candidate_params
self.pop_size = pop_size
self.n_generations = n_generations
Expand Down Expand Up @@ -149,7 +149,7 @@ def _select_parents(self, candidates: list[Candidate], n_parents: int) -> list[C
for candidate, distance in zip(front, crowding_distance):
candidate.distance = distance
if len(parents) + len(front) > n_parents: # If adding this front exceeds num_parents
front = sorted(front, key=lambda c: c.distance, reverse=True)
front = sorted(front, key=lambda candidate: candidate.distance, reverse=True)
parents += front[:n_parents - len(parents)]
break
parents += front
Expand All @@ -169,7 +169,7 @@ def _make_new_pop(self, parents: list[Candidate], pop_size: int, gen:int) -> lis
Makes new population by creating children from parents.
We use tournament selection to select parents for crossover.
"""
sorted_parents = sorted(parents, key=lambda c: (c.rank, -c.distance))
sorted_parents = sorted(parents, key=lambda candidate: (candidate.rank, -candidate.distance))
children = []
for i in range(pop_size):
parent1, parent2 = self._tournament_selection(sorted_parents)
Expand Down Expand Up @@ -226,23 +226,23 @@ def _record_gen_results(self, gen: int, candidates: list[Candidate], save_path:
Save the pareto front to disk.
"""
# Save statistics of candidates
gen_results = [c.record_state() for c in candidates]
gen_results = [candidate.record_state() for candidate in candidates]
gen_results_df = pd.DataFrame(gen_results)
gen_results_df.to_csv(save_path / f"{gen}.csv", index=False)

# Save rank 1 candidate state dicts
(save_path / f"{gen}").mkdir(parents=True, exist_ok=True)
pareto_candidates = [c for c in candidates if c.rank == 1]
for c in pareto_candidates:
torch.save(c.state_dict(), save_path / f"{gen}" / f"{c.gen}_{c.cand_id}.pt")
pareto_candidates = [candidate for candidate in candidates if candidate.rank == 1]
for candidate in pareto_candidates:
torch.save(candidate.state_dict(), save_path / f"{gen}" / f"{candidate.gen}_{candidate.cand_id}.pt")

def _record_candidate_avgs(self, gen: int, candidates: list[Candidate]) -> dict:
"""
Gets the average eluc and change for a population of candidates.
"""
avg_eluc = np.mean([c.metrics[0] for c in candidates])
avg_change = np.mean([c.metrics[1] for c in candidates])
return {"gen": gen, "eluc": avg_eluc, "change": avg_change}
return {"gen": gen, "eluc": avg_eluc, "change": avg_change}

def prescribe_land_use(self, context_df: pd.DataFrame, **kwargs) -> pd.DataFrame:
"""
Expand Down
1 change: 1 addition & 0 deletions use_cases/eluc/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ traitlets==5.9.0
transformers==4.31.0
typing_extensions==4.6.2
uri-template==1.2.0
wcwidth==0.2.13
webcolors==1.13
webencodings==0.5.1
websocket-client==1.5.3
Expand Down
4 changes: 2 additions & 2 deletions use_cases/eluc/tests/test_nsga2.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def get_fields(df):
Dummy fields method to create the encoder for dummy data.
"""
fields_df = df[constants.CAO_MAPPING["context"] + constants.CAO_MAPPING["actions"] + ["ELUC"]].astype("float64")
fields = dict()
fields = {}
for col in constants.CAO_MAPPING["context"] + constants.CAO_MAPPING["actions"] + ["ELUC"]:
# Set range of land and diff land uses manually to their true ranges because they
# do not need to be scaled
Expand Down Expand Up @@ -52,7 +52,7 @@ def get_fields(df):
"valued": "CONTINUOUS"
}

return fields
return fields

class TestTorchPrescriptor(unittest.TestCase):
"""
Expand Down
Loading