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

Add mask albedolandcover #1673

Merged
merged 7 commits into from Jul 22, 2020
Merged

Add mask albedolandcover #1673

merged 7 commits into from Jul 22, 2020

Conversation

bascrezee
Copy link
Contributor

@bascrezee bascrezee commented Jun 8, 2020

Adding a mask to mask physically impossible values of albedo.

@bascrezee bascrezee requested a review from qlejeune Jun 8, 2020
@qlejeune qlejeune marked this pull request as ready for review Jun 9, 2020
@bouweandela
Copy link
Member

bouweandela commented Jul 8, 2020

@qlejeune Do you approve these changes? If so, could you please click the 'Files changed' tab above, click the green 'Review changes' button, select 'Approve' and click 'Submit review'?

@qlejeune
Copy link
Contributor

qlejeune commented Jul 8, 2020

I approve the addition of the masking of physically not plausible albedo values. As discussed with @bascrezee, further masking may be added later on.

@bouweandela
Copy link
Member

bouweandela commented Jul 8, 2020

Thanks for reviewing! @mattiarighi Would you be able to run a final test and merge?

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 9, 2020

The recipe does not run, see logs:
main_log.txt
main_log_debug.txt

@bascrezee
Copy link
Contributor Author

bascrezee commented Jul 9, 2020

The recipe does not run, see logs:
main_log.txt
main_log_debug.txt

This is strange, it worked totally fine here two weeks ago. Did something recently change that could have broken my recipe? My initial guess looking at the logs is that someone implemented derivation of treeFrac, but here treeFrac is given on the file, so should not be derived. I will start testing myself as well.

@bascrezee
Copy link
Contributor Author

bascrezee commented Jul 9, 2020

Yes, the error must be related to very recent changes in ESMValCore, and is not related to this PR. @mattiarighi can you confirm that when checking out a recent branch from ESMValCore that does not include the very latest changes (e.g. git checkout add-lweGrace) it works fine.

I managed to reproduce the error that @mattiarighi gets when using the latest ESMValCore.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 9, 2020

Thank you. I'll try again with the latest release. That's what should be used for the final testing anyway.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 9, 2020

Tested with ESMValCore branch v2.0.0 (i.e., the "release candidate") I get a different error:
log.txt

@bascrezee
Copy link
Contributor Author

bascrezee commented Jul 9, 2020

Tested with ESMValCore branch v2.0.0 (i.e., the "release candidate") I get a different error:
log.txt

Also this error is new to me.

@bascrezee
Copy link
Contributor Author

bascrezee commented Jul 22, 2020

@mattiarighi Should work again.

@mattiarighi
Copy link
Contributor

mattiarighi commented Jul 22, 2020

It does. Waiting for the tests to complete.

@mattiarighi mattiarighi merged commit 43cfb16 into master Jul 22, 2020
1 check passed
@mattiarighi mattiarighi deleted the add-mask-albedolandcover branch Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants