Skip to content

Fix meltdown mitigation task 1 #2566

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 18 additions & 2 deletions exercises/concept/meltdown-mitigation/.docs/introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ if x > y:
elif y > z:
print("y is greater than x and z")
else:
print("z is great than x and y")
print("z is greater than x and y")
...
>>> z is great than x and y
>>> z is greater than x and y
Comment on lines +51 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this!

```

[Boolean operations][boolean operations] and [comparisons][comparisons] can be combined with conditionals for more complex testing:
Expand All @@ -74,6 +74,22 @@ else:
'13'
```

Bear in mind that in the special case where a function returns a boolean, it should not use an `if`.
Rather it should return the condition directly.

```python
# DO NOT use `if` and return `True`
>>> def is_leap_year(year):
if (year % 4 == 0 and year % 100 != 0) or year % 400 == 0:
return True
else:
return False

# instead return the condition directly
>>> def is_leap_year(year):
return (year % 4 == 0 and year % 100 != 0) or year % 400 == 0
```

Comment on lines +77 to +92
Copy link
Member

@BethanyG BethanyG Sep 8, 2021

Choose a reason for hiding this comment

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

While this is excellent information to discuss, I don't think this exercise (which is focused on conditionals) is the place to do it.

This note sets an exception/rule about Booleans without going into the nuance that one can return an expression directly without doing a check and assigning to an intermediary state variable -- which isn't just about conditionals and Booleans. The whole topic is better left to a mentor discussion/notes or explored in detail in an exercise on structuring functions/handling returns.

So this needs to be backed out of the exercise introduction. However, it would be good information to include in the about.md document (for display after the student completes the exercise), if you would like to PR it there.

Copy link
Author

Choose a reason for hiding this comment

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

I strongly disagree, unless someone can find a way to fix Task 1 so that it is not misleading (see another discussion).

The learning exercise on Booleans (Ghost Gobble Arcade Game) already has students returning expressions directly, and this concept is explained in the Basics exercise Guido's Gorgeous Lasagna.

Also, knowing when not to use a tool is part of mastering that tool.

[if statement]: https://docs.python.org/3/reference/compound_stmts.html#the-if-statement
[control flow tools]: https://docs.python.org/3/tutorial/controlflow.html#more-control-flow-tools
[truth value testing]: https://docs.python.org/3/library/stdtypes.html#truth-value-testing
Expand Down
28 changes: 9 additions & 19 deletions exercises/concept/meltdown-mitigation/.meta/exemplar.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,8 @@ def is_criticality_balanced(temperature, neutrons_emitted):
- The product of temperature and neutrons emitted per second less than 500000.
"""
output = temperature * neutrons_emitted
balanced = False

if (temperature < 800 and neutrons_emitted > 500) and output < 500000:
balanced = True

return balanced
return temperature < 800 and neutrons_emitted > 500 and output < 500000
Copy link
Member

Choose a reason for hiding this comment

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

As stated in the issue, this exercise is about conditionals, and every task should use or practice them - as contrived as that feels. This task in the exemplar now displays only the use of Booleans. If this task cannot be re-written to use/display the use of conditionals, then another task should be made or this task should be omitted from the exercise.

Copy link
Author

@punkstarman punkstarman Sep 9, 2021

Choose a reason for hiding this comment

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

As it is, the task is misleading, so

another task should be made or this task should be omitted from the exercise.

but I do not have the inspiration for this at the moment.



def reactor_efficiency(voltage, current, theoretical_max_power):
Expand All @@ -40,18 +36,15 @@ def reactor_efficiency(voltage, current, theoretical_max_power):
"""
generated_power = voltage * current
percentage_range = (generated_power / theoretical_max_power) * 100
efficiency_level = 'unknown'

if 80 <= percentage_range <= 100:
efficiency_level = 'green'
return 'green'
elif 60 <= percentage_range <= 79:
efficiency_level = 'orange'
return 'orange'
elif 30 <= percentage_range <= 59:
efficiency_level = 'red'
return 'red'
else:
efficiency_level = 'black'

return efficiency_level
return 'black'
Comment on lines 40 to +47
Copy link
Member

Choose a reason for hiding this comment

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

This task is getting revised via #2565, so I think we should back out these changes.



def fail_safe(temperature, neutrons_produced_per_second, threshold):
Expand All @@ -67,14 +60,11 @@ def fail_safe(temperature, neutrons_produced_per_second, threshold):
- `temperature * neutron per second` is not in the above-stated ranges == 'DANGER'
"""
output = temperature * neutrons_produced_per_second
operational_percentage = int((output / threshold) * 100)
safety_range = 'UNKNOWN'
operational_percentage = int(output / threshold) * 100

if operational_percentage < 40:
safety_range = 'LOW'
return 'LOW'
elif 90 <= operational_percentage <= 110:
safety_range = 'NORMAL'
return 'NORMAL'
else:
safety_range = 'DANGER'

return safety_range
return 'DANGER'